Merge pull request #695 from jetstack/disable-events-when-not-in-cluster

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

Author: wallrj-cyberark

cmd/agent_test.go

@@ -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")
-
 		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",
 			"--agent-config-file=testdata/agent/one-shot/success/config.yaml",
 			"--input-path=testdata/agent/one-shot/success/input.json",
 			"--output-path=/dev/null",

cmd/testdata/agent/one-shot/success/kubeconfig.yaml

@@ -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
 	}

pkg/agent/config.go

@@ -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")
 	})
 
@@ -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) {

pkg/agent/config_test.go

@@ -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)
 	}
@@ -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)
@@ -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