Merge pull request #602 from jetstack/VC-35738/feature

[VC-35738] Feature Branch

Author: wallrj

cmd/agent.go

@@ -7,7 +7,6 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/jetstack/preflight/pkg/agent"
-	"github.com/jetstack/preflight/pkg/logs"
 	"github.com/jetstack/preflight/pkg/permissions"
 )
 
@@ -16,7 +15,7 @@ var agentCmd = &cobra.Command{
 	Short: "start the preflight agent",
 	Long: `The agent will periodically gather data for the configured data
 	gatherers and send it to a remote backend for evaluation`,
-	Run: agent.Run,
+	RunE: agent.Run,
 }
 
 var agentInfoCmd = &cobra.Command{
@@ -34,24 +33,25 @@ var agentRBACCmd = &cobra.Command{
 	Use:   "rbac",
 	Short: "print the agent's minimal RBAC manifest",
 	Long:  `Print RBAC string by reading GVRs`,
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 
 		b, err := os.ReadFile(agent.Flags.ConfigFilePath)
 		if err != nil {
-			logs.Log.Fatalf("Failed to read config file: %s", err)
+			return fmt.Errorf("Failed to read config file: %s", err)
 		}
 		cfg, err := agent.ParseConfig(b)
 		if err != nil {
-			logs.Log.Fatalf("Failed to parse config file: %s", err)
+			return fmt.Errorf("Failed to parse config file: %s", err)
 		}
 
 		err = agent.ValidateDataGatherers(cfg.DataGatherers)
 		if err != nil {
-			logs.Log.Fatalf("Failed to validate data gatherers: %s", err)
+			return fmt.Errorf("Failed to validate data gatherers: %s", err)
 		}
 
 		out := permissions.GenerateFullManifest(cfg.DataGatherers)
 		fmt.Print(out)
+		return nil
 	},
 }
 

cmd/agent_test.go

@@ -0,0 +1,62 @@
+package cmd
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"os/exec"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestAgentRunOneShot runs the agent in `--one-shot` mode and verifies that it exits
+// 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",
+			"-v=1",
+		}
+		Execute()
+		return
+	}
+	t.Log("Running child process")
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestAgentRunOneShot$")
+	var (
+		stdout bytes.Buffer
+		stderr bytes.Buffer
+	)
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	cmd.Env = append(
+		os.Environ(),
+		"GO_CHILD=true",
+	)
+	err := cmd.Run()
+
+	stdoutStr := stdout.String()
+	stderrStr := stderr.String()
+	t.Logf("STDOUT\n%s\n", stdoutStr)
+	t.Logf("STDERR\n%s\n", stderrStr)
+	require.NoError(t, err, context.Cause(ctx))
+}

cmd/echo.go

@@ -11,7 +11,7 @@ var echoCmd = &cobra.Command{
 	Short: "starts an echo server to test the agent",
 	Long: `The agent sends data to a server. This echo server
 can be used to act as the server part and echo the data received by the agent.`,
-	Run: echo.Echo,
+	RunE: echo.Echo,
 }
 
 func init() {

cmd/root.go

@@ -1,12 +1,16 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"k8s.io/klog/v2"
+
+	"github.com/jetstack/preflight/pkg/logs"
 )
 
 // rootCmd represents the base command when called without any subcommands
@@ -17,6 +21,17 @@ var rootCmd = &cobra.Command{
 configuration checks using Open Policy Agent (OPA).
 
 Preflight checks are bundled into Packages`,
+	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+		return logs.Initialize()
+	},
+	// SilenceErrors and SilenceUsage prevents this command or any sub-command
+	// from printing arbitrary text to stderr.
+	// Why? To ensure that each line of output can be parsed as a single message
+	// for consumption by logging agents such as fluentd.
+	// Usage information is still available on stdout with the `-h` and `--help`
+	// flags.
+	SilenceErrors: true,
+	SilenceUsage:  true,
 }
 
 func init() {
@@ -27,11 +42,17 @@ func init() {
 
 // Execute adds all child commands to the root command and sets flags appropriately.
 // This is called by main.main(). It only needs to happen once to the rootCmd.
+// If the root command or sub-command returns an error, the error message will
+// will be logged and the process will exit with status 1.
 func Execute() {
-	if err := rootCmd.Execute(); err != nil {
-		fmt.Println(err)
-		os.Exit(1)
+	logs.AddFlags(rootCmd.PersistentFlags())
+	ctx := klog.NewContext(context.Background(), klog.Background())
+	var exitCode int
+	if err := rootCmd.ExecuteContext(ctx); err != nil {
+		exitCode = 1
+		klog.ErrorS(err, "Exiting due to error", "exit-code", exitCode)
 	}
+	klog.FlushAndExit(klog.ExitFlushTimeout, exitCode)
 }
 
 func setFlagsFromEnv(prefix string, fs *pflag.FlagSet) {

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

@@ -0,0 +1,4 @@
+# Just enough venafi-kubernetes-agent config to allow it to run with an input
+# file in one-shot mode.
+cluster_id: "venafi-kubernetes-agent-e2e"
+organization_id: "venafi-kubernetes-agent-e2e"

cmd/testdata/agent/one-shot/success/input.json

@@ -0,0 +1 @@
+[]

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

@@ -0,0 +1,15 @@
+# Just enough kubeconfig to satisfy client-go
+apiVersion: v1
+kind: Config
+current-context: cluster-1
+contexts:
+- name: cluster-1
+  context:
+    cluster: cluster-1
+    user: user-1
+clusters:
+- name: cluster-1
+  cluster:
+    server: https://192.0.2.1:8443
+preferences: {}
+users: []

go.mod

@@ -3,6 +3,7 @@ module github.com/jetstack/preflight
 go 1.22.0
 
 require (
+	github.com/Venafi/vcert/v5 v5.7.1
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/d4l3k/messagediff v1.2.1
 	github.com/fatih/color v1.17.0
@@ -22,13 +23,13 @@ require (
 	k8s.io/api v0.31.1
 	k8s.io/apimachinery v0.31.1
 	k8s.io/client-go v0.31.1
+	k8s.io/component-base v0.31.0
 	sigs.k8s.io/controller-runtime v0.19.0
 	sigs.k8s.io/yaml v1.4.0
 )
 
 require (
 	github.com/Khan/genqlient v0.7.0 // indirect
-	github.com/Venafi/vcert/v5 v5.7.1 // indirect
 	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
 	github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
 	github.com/aymerick/douceur v0.2.0 // indirect
@@ -37,6 +38,7 @@ require (
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
 	github.com/go-http-utils/headers v0.0.0-20181008091004-fed159eddc2a // indirect
+	github.com/go-logr/zapr v1.3.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/google/cel-go v0.20.1 // indirect
 	github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
@@ -62,7 +64,6 @@ require (
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	k8s.io/apiextensions-apiserver v0.31.0 // indirect
 	k8s.io/apiserver v0.31.0 // indirect
-	k8s.io/component-base v0.31.0 // indirect
 )
 
 require (
@@ -100,7 +101,7 @@ require (
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1
-	k8s.io/klog/v2 v2.130.1 // indirect
+	k8s.io/klog/v2 v2.130.1
 	k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f // indirect
 	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

hack/e2e/test.sh

@@ -191,14 +191,11 @@ EOF
 envsubst <application-team-1.yaml | kubectl apply -f -
 kubectl -n team-1 wait certificate app-0 --for=condition=Ready
 
-# Wait for log message indicating success.
-# Filter out distracting data gatherer errors and warnings.
-# Show other useful log messages on stderr.
+# Wait 60s for log message indicating success.
+# Parse logs as JSON using jq to ensure logs are all JSON formatted.
 # Disable pipefail to prevent SIGPIPE (141) errors from tee
 # See https://unix.stackexchange.com/questions/274120/pipe-fail-141-when-piping-output-into-tee-why
-set +o pipefail
 kubectl logs deployments/venafi-kubernetes-agent \
-  --follow \
-  --namespace venafi \
-  | tee >(grep -v -e "reflector\.go" -e "datagatherer" -e "data gatherer" >/dev/stderr) \
-  | grep -q "Data sent successfully"
+        --follow \
+        --namespace venafi \
+    | timeout 60 jq 'if .msg | test("Data sent successfully") then . | halt_error(0) end'

hack/e2e/values.venafi-kubernetes-agent.yaml

@@ -6,3 +6,6 @@ config:
 authentication:
   venafiConnection:
     enabled: true
+
+extraArgs:
+- --logging-format=json