Skip to content

[VC-43403] Disable events when not in cluster and only require installNamespace when using venafi-connection mode #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wallrj-cyberark merged 1 commit into from
Aug 22, 2025
Merged

[VC-43403] Disable events when not in cluster and only require installNamespace when using venafi-connection mode #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions cmd/agent_test.go
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@wallrj-cyberark wallrj-cyberark Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

$ go test ./cmd/...  -v -count 1
=== RUN   TestAgentRunOneShot
    agent_test.go:32: Running child process
    agent_test.go:50: STDOUT
        I0821 16:50:33.617894  135769 run.go:58] "Starting" logger="Run" version="development" commit=""
        I0821 16:50:33.618071  135769 config.go:491] "Configured to push to Venafi" logger="Run" mode="Jetstack Secure API Token" reason="--api-token was specified"
        I0821 16:50:33.618111  135769 run.go:116] "Healthz endpoints enabled" logger="Run.APIServer" addr=":8081" path="/healthz"
        I0821 16:50:33.618123  135769 run.go:120] "Readyz endpoints enabled" logger="Run.APIServer" addr=":8081" path="/readyz"
        I0821 16:50:33.618140  135769 run.go:269] "Pod event recorder disabled" logger="Run" reason="The agent does not appear to be running in a Kubernetes cluster." detail="When running in a Kubernetes cluster the following environment variables must be set: POD_NAME, POD_NODE, POD_UID, POD_NAMESPACE"
        I0821 16:50:33.618149  135769 run.go:308] "Reading data from local file" logger="Run.gatherAndOutputData" inputPath="testdata/agent/one-shot/success/input.json"
        I0821 16:50:33.618224  135769 run.go:334] "Data saved to local file" logger="Run.gatherAndOutputData" outputPath="/dev/null"
        I0821 16:50:33.618246  135769 run.go:476] "Starting" logger="Run.APIServer.ListenAndServe" addr=":8081"
        I0821 16:50:33.618250  135769 run.go:490] "Shutting down" logger="Run.APIServer.ListenAndServe" addr=":8081"
        I0821 16:50:33.618295  135769 run.go:505] "Shutdown complete" logger="Run.APIServer.ListenAndServe" addr=":8081"

    agent_test.go:51: STDERR

--- PASS: TestAgentRunOneShot (0.03s)
PASS
ok      github.com/jetstack/preflight/cmd       0.064s

Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,12 @@ import (
// after the first data gathering iteration.
func TestAgentRunOneShot(t *testing.T) {
if _, found := os.LookupEnv("GO_CHILD"); found {
// Silence the warning about missing pod name for event generation
// TODO(wallrj): This should not be required when an `--input-file` has been supplied.
t.Setenv("POD_NAME", "venafi-kubernetes-e2e")
// Silence the error about missing kubeconfig.
// TODO(wallrj): This should not be required when an `--input-file` has been supplied.
t.Setenv("KUBECONFIG", "testdata/agent/one-shot/success/kubeconfig.yaml")
Copy link
Copy Markdown
Contributor Author

@wallrj-cyberark wallrj-cyberark Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These deleted TODO comments were slightly inaccurate. What I meant was...if the agent is not running in a cluster, there's no Pod to attach events to, so the POD_NAME is not necessary.
And if the data is being read from a file, there are no K8S datagatherers, so KUBECONFIG should not be necessary. Nor is there a venafi-connection-lib to initialize, which is another reason KUBECONFIG is unnecessary.


os.Args = []string{
"preflight",
"agent",
"--one-shot",
// TODO(wallrj): This should not be required when an `--input-file` has been supplied.
"--api-token=should-not-be-required",
// TODO(wallrj): This should not be required when an `--input-file` has been supplied.
"--install-namespace=default",
Copy link
Copy Markdown
Contributor Author

@wallrj-cyberark wallrj-cyberark Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this deleted TODO comment should have said...--install-namespace is only necessary for initializing the venafi-connection-lib client, so it should not be necessary in this test.

"--agent-config-file=testdata/agent/one-shot/success/config.yaml",
"--input-path=testdata/agent/one-shot/success/input.json",
"--output-path=/dev/null",
Expand Down
15 changes: 0 additions & 15 deletions cmd/testdata/agent/one-shot/success/kubeconfig.yaml
View file
Open in desktop

This file was deleted.

20 changes: 12 additions & 8 deletions pkg/agent/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,22 +581,26 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
}

// Validation of --install-namespace.
installNS := flags.InstallNS
if flags.InstallNS == "" {
var err error
installNS, err = getInClusterNamespace()
if err != nil {
errs = multierror.Append(errs, fmt.Errorf("could not guess which namespace the agent is running in: %w", err))
{
installNS := flags.InstallNS
if installNS == "" {
var err error
installNS, err = getInClusterNamespace()
if err != nil {
if res.TLSPKMode == VenafiCloudVenafiConnection {
errs = multierror.Append(errs, fmt.Errorf("could not guess which namespace the agent is running in: %w", err))
}
}
}
res.InstallNS = installNS
}
res.InstallNS = installNS

// Validation of --venafi-connection and --venafi-connection-namespace.
if res.TLSPKMode == VenafiCloudVenafiConnection {
res.VenConnName = flags.VenConnName
venConnNS := flags.VenConnNS
if flags.VenConnNS == "" {
venConnNS = installNS
venConnNS = res.InstallNS
}
res.VenConnNS = venConnNS
}
Expand Down
32 changes: 30 additions & 2 deletions pkg/agent/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
// OAuth mode.
fakeCredsPath := withFile(t, `{"user_id":"foo","user_secret":"bar","client_id": "baz","client_secret": "foobar","auth_server_domain":"bazbar"}`)

t.Run("--install-namespace must be provided if POD_NAMESPACE is not set", func(t *testing.T) {
t.Run("In Venafi Connection mode, --install-namespace must be provided if POD_NAMESPACE is not set", func(t *testing.T) {
_, _, err := ValidateAndCombineConfig(discardLogs(),
withConfig(testutil.Undent(`
server: https://api.venafi.eu
organization_id: foo
cluster_id: bar
period: 5m
`)),
withCmdLineFlags("--credentials-file", fakeCredsPath))
withCmdLineFlags("--venafi-connection", "venafi-components"))
assert.EqualError(t, err, "1 error occurred:\n\t* could not guess which namespace the agent is running in: POD_NAMESPACE env var not set, meaning that you are probably not running in cluster. Please use --install-namespace or POD_NAMESPACE to specify the namespace in which the agent is running.\n\n")
})

Expand Down Expand Up @@ -615,6 +615,34 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, VenafiCloudVenafiConnection, got.TLSPKMode)
})

// When --input-path is supplied, the data is being read from a local file
// and the agent is probably running outside the cluster and has no access
// to a cluster, so the environment variables which are required for
// generating events attached to the Agent pod should not be required:
// POD_NAME, POD_NAMESPACE, POD_UID, KUBECONFIG, etc.
// This test deliberately does not set those environment variables.
//
// TODO(wallrj): Some other config settings like cluster_id, organization_id
// should also not be required in this situation. We'll fix those in the
// future.
t.Run("--input-path requires no Kubernetes config", func(t *testing.T) {
expectedInputPath := "/foo/bar/baz"
got, _, err := ValidateAndCombineConfig(discardLogs(),
withConfig(testutil.Undent(`
cluster_id: should-not-be-required
organization_id: should-not-be-required
`)),
withCmdLineFlags(
"--one-shot",
"--input-path", expectedInputPath,
"--output-path", "/dev/null",
"--api-token", "should-not-be-required",
),
)
require.NoError(t, err)
assert.Equal(t, expectedInputPath, got.InputPath)
})
}

func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
Expand Down
41 changes: 24 additions & 17 deletions pkg/agent/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func Run(cmd *cobra.Command, args []string) (returnErr error) {

// To help users notice issues with the agent, we show the error messages in
// the agent pod's events.
eventf, err := newEventf(log, config.InstallNS)
eventf, err := newEventf(log)
if err != nil {
return fmt.Errorf("failed to create event recorder: %v", err)
}
Expand Down Expand Up @@ -260,7 +260,19 @@ func Run(cmd *cobra.Command, args []string) (returnErr error) {
// POD_NAME to contain the pod name. Note that the RBAC rule allowing sending
// events is attached to the pod's service account, not the impersonated service
// account (venafi-connection).
func newEventf(log logr.Logger, installNS string) (Eventf, error) {
func newEventf(log logr.Logger) (Eventf, error) {
podName := os.Getenv("POD_NAME")
podNode := os.Getenv("POD_NODE")
podUID := os.Getenv("POD_UID")
podNamespace := os.Getenv("POD_NAMESPACE")
if podName == "" || podNode == "" || podUID == "" || podNamespace == "" {
log.Info(
"Pod event recorder disabled",
"reason", "The agent does not appear to be running in a Kubernetes cluster.",
"detail", "When running in a Kubernetes cluster the following environment variables must be set: POD_NAME, POD_NODE, POD_UID, POD_NAMESPACE",
)
return func(eventType, reason, msg string, args ...interface{}) {}, nil
}
restcfg, err := kubeconfig.LoadRESTConfig("")
if err != nil {
return nil, fmt.Errorf("failed to load kubeconfig: %v", err)
Expand All @@ -269,22 +281,17 @@ func newEventf(log logr.Logger, installNS string) (Eventf, error) {
_ = corev1.AddToScheme(scheme)

var eventf Eventf
if os.Getenv("POD_NAME") == "" {
eventf = func(eventType, reason, msg string, args ...interface{}) {}
log.Error(nil, "Error messages will not show in the pod's events because the POD_NAME environment variable is empty")
} else {
podName := os.Getenv("POD_NAME")

eventClient, err := kubernetes.NewForConfig(restcfg)
if err != nil {
return eventf, fmt.Errorf("failed to create event client: %v", err)
}
broadcaster := record.NewBroadcaster()
broadcaster.StartRecordingToSink(&clientgocorev1.EventSinkImpl{Interface: eventClient.CoreV1().Events(installNS)})
eventRec := broadcaster.NewRecorder(scheme, corev1.EventSource{Component: "venafi-kubernetes-agent", Host: os.Getenv("POD_NODE")})
eventf = func(eventType, reason, msg string, args ...interface{}) {
eventRec.Eventf(&corev1.Pod{ObjectMeta: v1.ObjectMeta{Name: podName, Namespace: installNS, UID: types.UID(os.Getenv("POD_UID"))}}, eventType, reason, msg, args...)
}
eventClient, err := kubernetes.NewForConfig(restcfg)
if err != nil {
return eventf, fmt.Errorf("failed to create event client: %v", err)
}
broadcaster := record.NewBroadcaster()
broadcaster.StartRecordingToSink(&clientgocorev1.EventSinkImpl{Interface: eventClient.CoreV1().Events(podNamespace)})
eventRec := broadcaster.NewRecorder(scheme, corev1.EventSource{Component: "venafi-kubernetes-agent", Host: podNode})
eventf = func(eventType, reason, msg string, args ...interface{}) {
eventRec.Eventf(&corev1.Pod{ObjectMeta: v1.ObjectMeta{Name: podName, Namespace: podNamespace, UID: types.UID(podUID)}}, eventType, reason, msg, args...)

}

return eventf, nil
Expand Down