adding metrics-require-rbac flag, metrics-secure validation logic and corresponding scaffolding logic (#119)

Signed-off-by: Adam D. Cornett <[email protected]>
Co-authored-by: Adam D. Cornett <[email protected]>

Author: openshift-cherrypick-robot

go.mod

@@ -36,12 +36,15 @@ require (
 	github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/blang/semver/v4 v4.0.0 // indirect
+	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/emicklei/go-restful/v3 v3.11.2 // indirect
 	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.0 // indirect
+	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
+	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect
 	github.com/go-openapi/jsonpointer v0.20.2 // indirect
 	github.com/go-openapi/jsonreference v0.20.4 // indirect
@@ -57,6 +60,7 @@ require (
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect
 	github.com/h2non/filetype v1.1.3 // indirect
 	github.com/h2non/go-is-svg v0.0.0-20160927212452-35e8c4b0612c // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
@@ -84,6 +88,14 @@ require (
 	github.com/spf13/cast v1.6.0 // indirect
 	github.com/stoewer/go-strcase v1.3.0 // indirect
 	github.com/subosito/gotenv v1.6.0 // indirect
+	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect
+	go.opentelemetry.io/otel v1.24.0 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.23.1 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.23.1 // indirect
+	go.opentelemetry.io/otel/metric v1.24.0 // indirect
+	go.opentelemetry.io/otel/sdk v1.23.1 // indirect
+	go.opentelemetry.io/otel/trace v1.24.0 // indirect
+	go.opentelemetry.io/proto/otlp v1.1.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.26.0 // indirect
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
@@ -104,8 +116,11 @@ require (
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
+	k8s.io/apiserver v0.30.5 // indirect
+	k8s.io/component-base v0.30.5 // indirect
 	k8s.io/klog/v2 v2.120.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
+	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )

go.sum

@@ -86,6 +86,7 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk
 github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
+github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
 github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
@@ -128,7 +129,6 @@ github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyE
 github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w=
 github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
 github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
-github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 h1:/c3QmbOGMGTOumP2iT/rCwB7b0QDGLKzqOmktBjT+Is=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1/go.mod h1:5SN9VR2LTsRFsrEC6FHgRbTWrTHu6tqPeKxEQv15giM=
 github.com/h2non/filetype v1.1.3 h1:FKkx9QbD7HR/zjK1Ia5XiBsq9zdLi5Kf3zGyFTAFkGg=

hack/generate/samples/ansible/constants.go

@@ -516,6 +516,17 @@ const rolesForBaseOperator = `
       - patch
       - update
       - watch
+  ##
+  ## Apply metrics_reader role for testing
+  ## This should align with metrics_reader.yaml
+  ## scaffolded by kubebuilder, but by default
+  ## that role is not applied to the generated
+  ## controller, so we apply it here.
+  ##
+  - nonResourceURLs:
+    - "/metrics"
+    verbs:
+    - get
 #+kubebuilder:scaffold:rules
 `
 
@@ -526,12 +537,14 @@ const customMetricsTest = `
     label_selectors:
       - "control-plane = controller-manager"
   register: output
+
 - name: Curl the metrics from the manager
   kubernetes.core.k8s_exec:
-    namespace: default
+    namespace: "{{ output.resources[0].metadata.namespace }}"
     container: manager
     pod: "{{ output.resources[0].metadata.name }}"
-    command: curl localhost:8443/metrics
+    command: >
+      bash -c 'curl -k -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://localhost:8443/metrics'
   register: metrics_output
 
 - name: Assert sanity metrics were created

internal/ansible/flags/flag.go

@@ -22,6 +22,7 @@ import (
 	"github.com/spf13/pflag"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
@@ -48,6 +49,7 @@ type Flags struct {
 	ProxyPort                  int
 	EnableHTTP2                bool
 	SecureMetrics              bool
+	MetricsRequireRBAC         bool
 
 	// If not nil, used to deduce which flags were set in the CLI.
 	flagSet *pflag.FlagSet
@@ -194,12 +196,16 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		false,
 		"enables HTTP/2 on the webhook and metrics servers",
 	)
-
 	flagSet.BoolVar(&f.SecureMetrics,
 		"metrics-secure",
 		false,
 		"enables secure serving of the metrics endpoint",
 	)
+	flagSet.BoolVar(&f.MetricsRequireRBAC,
+		"metrics-require-rbac",
+		false,
+		"enables protection of the metrics endpoint with RBAC-based authn/authz."+
+			"see https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/filters#WithAuthenticationAndAuthorization for more info")
 }
 
 // ToManagerOptions uses the flag set in f to configure options.
@@ -254,5 +260,14 @@ func (f *Flags) ToManagerOptions(options manager.Options) manager.Options {
 		options.Metrics.TLSOpts = append(options.Metrics.TLSOpts, disableHTTP2)
 	}
 	options.Metrics.SecureServing = f.SecureMetrics
+
+	if f.MetricsRequireRBAC {
+		// FilterProvider is used to protect the metrics endpoint with authn/authz.
+		// These configurations ensure that only authorized users and service accounts
+		// can access the metrics endpoint. The RBAC are configured in 'config/rbac/kustomization.yaml'. More info:
+		// https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/filters#WithAuthenticationAndAuthorization
+		options.Metrics.FilterProvider = filters.WithAuthenticationAndAuthorization
+	}
+
 	return options
 }

internal/cmd/ansible-operator/run/cmd.go

@@ -75,6 +75,12 @@ func NewCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "run",
 		Short: "Run the operator",
+		Args: func(cmd *cobra.Command, args []string) error {
+			if cmd.Flag("metrics-require-rbac").Value.String() == "true" && cmd.Flag("metrics-secure").Value.String() == "false" {
+				return errors.New("--metrics-secure flag is required when --metrics-require-rbac is present")
+			}
+			return nil
+		},
 		Run: func(cmd *cobra.Command, _ []string) {
 			logf.SetLogger(zapf.New(zapf.UseFlagOptions(opts)))
 			run(cmd, f)

pkg/plugins/ansible/v1/init.go

@@ -160,6 +160,7 @@ func addInitCustomizations(projectName string) error {
 	}
 
 	managerFile := filepath.Join("config", "manager", "manager.yaml")
+	managerMetricsPatch := filepath.Join("config", "default", "manager_metrics_patch.yaml")
 
 	// todo: we ought to use afero instead. Replace this methods to insert/update
 	// by https://github.com/kubernetes-sigs/kubebuilder/pull/2119
@@ -173,6 +174,27 @@ func addInitCustomizations(projectName string) error {
 		return err
 	}
 
+	// Enable the proper auth/metrics flags
+	err = util.ReplaceInFile(managerMetricsPatch,
+		`# This patch adds the args to allow exposing the metrics endpoint using HTTPS
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-bind-address=:8443`, `# This patch adds the args to allow exposing the metrics endpoint using HTTPS
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-bind-address=:8443
+# This patch adds the args to allow securing the metrics endpoint
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-secure
+# This patch adds the args to allow RBAC-based authn/authz the metrics endpoint
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-require-rbac`)
+	if err != nil {
+		return err
+	}
+
 	// update default resource request and limits with bigger values
 	const resourcesLimitsFragment = `  resources:
           limits:

pkg/testutils/e2e/metrics/helpers.go

@@ -1,25 +1,60 @@
 package metrics
 
 import (
+	"encoding/base64"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 
 	"github.com/operator-framework/ansible-operator-plugins/pkg/testutils/kubernetes"
 	"github.com/operator-framework/ansible-operator-plugins/pkg/testutils/sample"
+	"github.com/operator-framework/ansible-operator-plugins/test/common"
 )
 
 // GetMetrics creates a pod with the permissions to `curl` metrics. It will then return the output of the `curl` pod
 func GetMetrics(sample sample.Sample, kubectl kubernetes.Kubectl, metricsClusterRoleBindingName string) string {
+	ginkgo.By("granting permissions to access the metrics and read the token")
+	out, err := kubectl.Command("create", "clusterrolebinding", metricsClusterRoleBindingName,
+		fmt.Sprintf("--clusterrole=%s-metrics-reader", sample.Name()),
+		fmt.Sprintf("--serviceaccount=%s:%s", kubectl.Namespace(), kubectl.ServiceAccount()))
+	fmt.Println("OUT --", out)
+	gomega.Expect(err).NotTo(gomega.HaveOccurred())
+
+	// As of Kubernetes 1.24 a ServiceAccount no longer has a ServiceAccount token secret autogenerated. We have to create it manually here
+	ginkgo.By("Creating the ServiceAccount token")
+	secretFile, err := common.GetSASecret(kubectl.ServiceAccount(), sample.Dir())
+	gomega.Expect(err).NotTo(gomega.HaveOccurred())
+	gomega.Eventually(func() error {
+		out, err = kubectl.Apply(true, "-f", secretFile)
+		fmt.Println("OUT -- ", out)
+		return err
+	}, time.Minute, time.Second).Should(gomega.Succeed())
+
+	ginkgo.By("reading the metrics token")
+	// Filter token query by service account in case more than one exists in a namespace.
+	query := fmt.Sprintf(`{.items[?(@.metadata.annotations.kubernetes\.io/service-account\.name=="%s")].data.token}`,
+		kubectl.ServiceAccount(),
+	)
+	out, err = kubectl.Get(true, "secrets")
+	gomega.Expect(err).NotTo(gomega.HaveOccurred())
+	fmt.Println("OUT --", out)
+	b64Token, err := kubectl.Get(true, "secrets", "-o=jsonpath="+query)
+	fmt.Println("OUT--", b64Token)
+	gomega.Expect(err).NotTo(gomega.HaveOccurred())
+	token, err := base64.StdEncoding.DecodeString(strings.TrimSpace(b64Token))
+	gomega.Expect(err).NotTo(gomega.HaveOccurred())
+	gomega.Expect(len(token)).To(gomega.BeNumerically(">", 0))
+
 	ginkgo.By("creating a curl pod")
 	cmdOpts := []string{
 		"run", "curl", "--image=curlimages/curl:7.68.0", "--restart=OnFailure", "--",
-		"curl", "-v",
-		fmt.Sprintf("http://%s-controller-manager-metrics-service.%s.svc:8443/metrics", sample.Name(), kubectl.Namespace()),
+		"curl", "-v", "-k", "-H", fmt.Sprintf(`Authorization: Bearer %s`, token),
+		fmt.Sprintf("https://%s-controller-manager-metrics-service.%s.svc:8443/metrics", sample.Name(), kubectl.Namespace()),
 	}
-	out, err := kubectl.CommandInNamespace(cmdOpts...)
+	out, err = kubectl.CommandInNamespace(cmdOpts...)
 	fmt.Println("OUT --", out)
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())
 
@@ -58,5 +93,10 @@ func CleanUpMetrics(kubectl kubernetes.Kubectl, metricsClusterRoleBindingName st
 		return fmt.Errorf("encountered an error when deleting the metrics pod: %w", err)
 	}
 
+	_, err = kubectl.Delete(false, "clusterrolebinding", metricsClusterRoleBindingName)
+	if err != nil {
+		return fmt.Errorf("encountered an error when deleting the metrics clusterrolebinding: %w", err)
+	}
+
 	return nil
 }

test/e2e/ansible/cluster_test.go

@@ -57,6 +57,9 @@ var _ = Describe("Running ansible projects", func() {
 		})
 
 		AfterEach(func() {
+			By("deleting curl pod")
+			testutils.WrapWarnOutput(kctl.Delete(false, "pod", "curl"))
+
 			By("cleaning up metrics")
 			Expect(metrics.CleanUpMetrics(kctl, metricsClusterRoleBindingName)).To(Succeed())
 

testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml

@@ -2,3 +2,11 @@
 - op: add
   path: /spec/template/spec/containers/0/args/0
   value: --metrics-bind-address=:8443
+# This patch adds the args to allow securing the metrics endpoint
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-secure
+# This patch adds the args to allow RBAC-based authn/authz the metrics endpoint
+- op: add
+  path: /spec/template/spec/containers/0/args/0
+  value: --metrics-require-rbac

testdata/memcached-molecule-operator/config/rbac/role.yaml

@@ -122,5 +122,16 @@ rules:
       - patch
       - update
       - watch
+  ##
+  ## Apply metrics_reader role for testing
+  ## This should align with metrics_reader.yaml
+  ## scaffolded by kubebuilder, but by default
+  ## that role is not applied to the generated
+  ## controller, so we apply it here.
+  ##
+  - nonResourceURLs:
+    - "/metrics"
+    verbs:
+    - get
 #+kubebuilder:scaffold:rules
 

testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml

@@ -73,12 +73,14 @@
     label_selectors:
       - "control-plane = controller-manager"
   register: output
+
 - name: Curl the metrics from the manager
   kubernetes.core.k8s_exec:
-    namespace: default
+    namespace: "{{ output.resources[0].metadata.namespace }}"
     container: manager
     pod: "{{ output.resources[0].metadata.name }}"
-    command: curl localhost:8443/metrics
+    command: >
+      bash -c 'curl -k -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://localhost:8443/metrics'
   register: metrics_output
 
 - name: Assert sanity metrics were created