fix: make inline auth work in direct mode too (#520)

## Issue
Resolves
#517.

## Description
The original implementation of inline auth only worked in continuous
mode because the code that sets env vars for the AWS SDK was in
`Reconcile` and was therefore not invoked by `Validate` (which direct
mode invokes). This fixes it by moving that code to `Validate` so that
it will run during direct mode too.

Also makes the following minor changes:

* Renames `accessKeyPair` in the YAML to `credentials` to match other
plugins and leave room for other credential related data.
* Removes code that reads values for the `AWS_SESSION_TOKEN` env var
from secrets and inline config and sets the env var for this. This env
var isn't meant to be manually set. Instead, a role is meant to be
assumed. Our spec already allows STS to be enabled and a role ARN to be
configured when it's enabled.

---------

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>

Author: Matt Welke

api/v1alpha1/awsvalidator_types.go

@@ -78,8 +78,8 @@ type AwsAuth struct {
 	// The secret data's keys and values are expected to align with valid AWS environment variable credentials,
 	// per the options defined in https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/#environment-variables.
 	SecretName string `json:"secretName,omitempty" yaml:"secretName,omitempty"`
-	// The credentials to use when running in direct mode. If provided, fields Implicit and SecretName are ignored.
-	Credentials *Credentials `json:"accessKeyPair,omitempty" yaml:"accessKeyPair,omitempty"`
+	// The credentials to use when running in direct mode.
+	Credentials *Credentials `json:"credentials,omitempty" yaml:"credentials,omitempty"`
 	// STS authentication properties (optional)
 	StsAuth *AwsSTSAuth `json:"stsAuth,omitempty" yaml:"stsAuth,omitempty"`
 }
@@ -90,8 +90,6 @@ type Credentials struct {
 	AccessKeyID string `json:"accessKeyId" yaml:"accessKeyId"`
 	// The secret access key of an access key pair.
 	SecretAccessKey string `json:"secretAccessKey" yaml:"secretAccessKey"`
-	// The session token if one is being used.
-	SessionToken *string `json:"sessionToken" yaml:"sessionToken"`
 }
 
 // AwsSTSAuth defines AWS STS authentication configuration for an AwsValidator.

api/v1alpha1/zz_generated.deepcopy.go

@@ -89,23 +89,18 @@ spec:
               auth:
                 description: AwsAuth defines authentication configuration for an AwsValidator.
                 properties:
-                  accessKeyPair:
+                  credentials:
                     description: The credentials to use when running in direct mode.
-                      If provided, fields Implicit and SecretName are ignored.
                     properties:
                       accessKeyId:
                         description: The access key ID of an access key pair.
                         type: string
                       secretAccessKey:
                         description: The secret access key of an access key pair.
                         type: string
-                      sessionToken:
-                        description: The session token if one is being used.
-                        type: string
                     required:
                     - accessKeyId
                     - secretAccessKey
-                    - sessionToken
                     type: object
                   implicit:
                     description: |-

chart/validator-plugin-aws/crds/validation.spectrocloud.labs_awsvalidators.yaml

@@ -89,23 +89,18 @@ spec:
               auth:
                 description: AwsAuth defines authentication configuration for an AwsValidator.
                 properties:
-                  accessKeyPair:
+                  credentials:
                     description: The credentials to use when running in direct mode.
-                      If provided, fields Implicit and SecretName are ignored.
                     properties:
                       accessKeyId:
                         description: The access key ID of an access key pair.
                         type: string
                       secretAccessKey:
                         description: The secret access key of an access key pair.
                         type: string
-                      sessionToken:
-                        description: The session token if one is being used.
-                        type: string
                     required:
                     - accessKeyId
                     - secretAccessKey
-                    - sessionToken
                     type: object
                   implicit:
                     description: |-

config/crd/bases/validation.spectrocloud.labs_awsvalidators.yaml

@@ -17,13 +17,12 @@ require (
 	github.com/go-logr/logr v1.4.2
 	github.com/onsi/ginkgo/v2 v2.22.0
 	github.com/onsi/gomega v1.35.1
-	github.com/pkg/errors v0.9.1
+	github.com/stretchr/testify v1.9.0
 	github.com/validator-labs/validator v0.1.13
 	golang.org/x/exp v0.0.0-20241108190413-2d47ceb2692f
 	k8s.io/api v0.31.3
 	k8s.io/apimachinery v0.31.3
 	k8s.io/client-go v0.31.3
-	k8s.io/utils v0.0.0-20241104163129-6fe5fd82f078
 	sigs.k8s.io/cluster-api v1.8.5
 	sigs.k8s.io/controller-runtime v0.19.2
 )
@@ -71,6 +70,8 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.19.1 // indirect
 	github.com/prometheus/client_model v0.6.1 // indirect
 	github.com/prometheus/common v0.55.0 // indirect
@@ -89,12 +90,14 @@ require (
 	golang.org/x/tools v0.27.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/protobuf v1.35.2 // indirect
+	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.31.0 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
+	k8s.io/utils v0.0.0-20241104163129-6fe5fd82f078 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 	sigs.k8s.io/yaml v1.4.0 // indirect

go.mod

@@ -20,16 +20,13 @@ package controller
 import (
 	"context"
 	"fmt"
-	"os"
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ktypes "k8s.io/apimachinery/pkg/types"
-	"k8s.io/utils/ptr"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -40,9 +37,10 @@ import (
 	vres "github.com/validator-labs/validator/pkg/validationresult"
 )
 
-var errInvalidAccessKeyID = errors.New("access key ID is invalid, must be a non-empty string")
-var errInvalidSecretAccessKey = errors.New("secret access key is invalid, must be a non-empty string")
-var errInvalidSessionToken = errors.New("session token is invalid, must be a non-empty string")
+const (
+	secretKeyAccessKeyID     = "AWS_ACCESS_KEY_ID"     // #nosec G101
+	secretKeySecretAccessKey = "AWS_SECRET_ACCESS_KEY" // #nosec G101
+)
 
 // AwsValidatorReconciler reconciles a AwsValidator object
 type AwsValidatorReconciler struct {
@@ -57,6 +55,7 @@ type AwsValidatorReconciler struct {
 
 // Reconcile reconciles each rule found in each AWSValidator in the cluster and creates ValidationResults accordingly
 func (r *AwsValidatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	var err error
 	l := r.Log.V(0).WithValues("name", req.Name, "namespace", req.Namespace)
 	l.Info("Reconciling AwsValidator")
 
@@ -68,9 +67,9 @@ func (r *AwsValidatorReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Ensure both AWS env vars are set.
-	if err := r.configureAwsAuth(validator.Spec.Auth, req.Namespace, l); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to set AWS auth env vars: %w", err)
+	// Override auth data in spec with auth data from Secret if applicable.
+	if validator.Spec.Auth, err = r.authFromSecret(validator.Spec.Auth, req.Namespace, l); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to get auth data from Secret: %w", err)
 	}
 
 	// Get the active validator's validation result
@@ -111,78 +110,46 @@ func (r *AwsValidatorReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	return ctrl.Result{RequeueAfter: time.Second * 120}, nil
 }
 
-// configureAwsAuth sets environment variables to control AWS authentication. Order of precedence
-// for source:
-// 1 - Kubernetes secret
-// 2 - Specified inline in spec
-// Returns an error if env vars couldn't be set for any reason.
-func (r *AwsValidatorReconciler) configureAwsAuth(auth v1alpha1.AwsAuth, reqNamespace string, l logr.Logger) error {
+// Checks whether the spec indicates that auth data should come from a k8s Secret instead of inline
+// auth. If so, all data must come from the Secret. If any is missing, returns an error. If all data
+// is present, overrides the data in the auth object.
+func (r *AwsValidatorReconciler) authFromSecret(auth v1alpha1.AwsAuth, reqNamespace string, l logr.Logger) (v1alpha1.AwsAuth, error) {
+	// If using implicit auth, there is no need to check for k8s Secrets.
 	if auth.Implicit {
-		l.Info("auth.implicit set to true. Skipping setting AWS env vars.")
-		return nil
+		l.Info("auth.implicit set to true. Skipping setting AWS_ env vars.")
+		return auth, nil
 	}
 
-	if auth.Credentials == nil {
-		auth.Credentials = &v1alpha1.Credentials{}
+	// Same if no secret name provided.
+	if auth.SecretName == "" {
+		l.Info("No Secret name provided. Skipping looking for Secret to override auth data.")
+		return auth, nil
 	}
 
-	// If Secret name provided, override any env var values with values from its data.
-	if auth.SecretName != "" {
-		l.Info("auth.secretName provided. Using Secret as source for any AWS env vars defined in its data.", "secretName", auth.SecretName, "secretNamespace", reqNamespace)
-		nn := ktypes.NamespacedName{Name: auth.SecretName, Namespace: reqNamespace}
-		secret := &corev1.Secret{}
-		if err := r.Get(context.Background(), nn, secret); err != nil {
-			return fmt.Errorf("failed to get Secret: %w", err)
-		}
-		if accessKeyID, ok := secret.Data["AWS_ACCESS_KEY_ID"]; ok {
-			l.Info("Using access key ID from Secret.")
-			auth.Credentials.AccessKeyID = string(accessKeyID)
-		}
-		if secretAccessKey, ok := secret.Data["AWS_SECRET_ACCESS_KEY"]; ok {
-			l.Info("Using secret access key from Secret.")
-			auth.Credentials.SecretAccessKey = string(secretAccessKey)
-		}
-		if sessionToken, ok := secret.Data["AWS_SESSION_TOKEN"]; ok {
-			l.Info("Using session token from Secret.")
-			auth.Credentials.SessionToken = ptr.To(string(sessionToken))
-		}
+	if auth.Credentials == nil {
+		auth.Credentials = &v1alpha1.Credentials{}
 	}
 
-	// Validate values collected from inline config and/or Secret. We can't rely on CRD validation
-	// for this because some of the values may have come from a Secret, and there is no way for the
-	// Kube API to validate content in its data.
-	if auth.Credentials.AccessKeyID == "" {
-		return errInvalidAccessKeyID
-	}
-	if auth.Credentials.SecretAccessKey == "" {
-		return errInvalidSecretAccessKey
-	}
-	if auth.Credentials.SessionToken != nil && *auth.Credentials.SessionToken == "" {
-		return errInvalidSessionToken
+	l.Info("auth.secretName provided. Using Secret as source for any AWS_ env vars defined in its data.", "secretName", auth.SecretName, "secretNamespace", reqNamespace)
+	nn := ktypes.NamespacedName{Name: auth.SecretName, Namespace: reqNamespace}
+	secret := &corev1.Secret{}
+	if err := r.Get(context.Background(), nn, secret); err != nil {
+		return auth, fmt.Errorf("failed to get Secret: %w", err)
 	}
 
-	// Log non-secret data for help with debugging. Don't log the secret access key.
-	nonSecretData := map[string]string{
-		"accesskeyId": auth.Credentials.AccessKeyID,
+	accessKeyID, ok := secret.Data[secretKeyAccessKeyID]
+	if !ok {
+		return v1alpha1.AwsAuth{}, fmt.Errorf("Key %s missing from Secret", secretKeyAccessKeyID)
 	}
-	l.Info("Determined AWS auth data.", "nonSecretData", nonSecretData)
+	auth.Credentials.AccessKeyID = string(accessKeyID)
 
-	// Use collected and validated values to set env vars.
-	data := map[string]string{
-		"AWS_ACCESS_KEY_ID":     auth.Credentials.AccessKeyID,
-		"AWS_SECRET_ACCESS_KEY": auth.Credentials.SecretAccessKey,
-	}
-	if auth.Credentials.SessionToken != nil {
-		data["AWS_SESSION_TOKEN"] = *auth.Credentials.SessionToken
-	}
-	for k, v := range data {
-		if err := os.Setenv(k, v); err != nil {
-			return err
-		}
-		r.Log.Info("Set environment variable", "envVar", k)
+	secretAccessKey, ok := secret.Data[secretKeySecretAccessKey]
+	if !ok {
+		return v1alpha1.AwsAuth{}, fmt.Errorf("Key %s missing from Secret", secretKeySecretAccessKey)
 	}
+	auth.Credentials.SecretAccessKey = string(secretAccessKey)
 
-	return nil
+	return auth, nil
 }
 
 // SetupWithManager sets up the controller with the Manager.