diff --git a/go.mod b/go.mod index 5a53dd2..b4cde3c 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 6c5ab92..fe8188c 100644 --- a/go.sum +++ b/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= diff --git a/hack/generate/samples/ansible/constants.go b/hack/generate/samples/ansible/constants.go index 2932922..40ce66b 100644 --- a/hack/generate/samples/ansible/constants.go +++ b/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 diff --git a/internal/ansible/flags/flag.go b/internal/ansible/flags/flag.go index ed28e85..04d2629 100644 --- a/internal/ansible/flags/flag.go +++ b/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/controller-runtime@v0.18.4/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/controller-runtime@v0.18.4/pkg/metrics/filters#WithAuthenticationAndAuthorization + options.Metrics.FilterProvider = filters.WithAuthenticationAndAuthorization + } + return options } diff --git a/internal/cmd/ansible-operator/run/cmd.go b/internal/cmd/ansible-operator/run/cmd.go index 1eeda68..a58ae98 100644 --- a/internal/cmd/ansible-operator/run/cmd.go +++ b/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) diff --git a/pkg/plugins/ansible/v1/init.go b/pkg/plugins/ansible/v1/init.go index ae558ed..f91c24a 100644 --- a/pkg/plugins/ansible/v1/init.go +++ b/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: diff --git a/pkg/testutils/e2e/metrics/helpers.go b/pkg/testutils/e2e/metrics/helpers.go index bbf915e..9c76fad 100644 --- a/pkg/testutils/e2e/metrics/helpers.go +++ b/pkg/testutils/e2e/metrics/helpers.go @@ -1,7 +1,9 @@ package metrics import ( + "encoding/base64" "fmt" + "strings" "time" "github.com/onsi/ginkgo/v2" @@ -9,17 +11,50 @@ import ( "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 } diff --git a/test/e2e/ansible/cluster_test.go b/test/e2e/ansible/cluster_test.go index e1ec788..95b43d7 100644 --- a/test/e2e/ansible/cluster_test.go +++ b/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()) diff --git a/testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml b/testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml index 2aaef65..a3cb2f1 100644 --- a/testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml +++ b/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 diff --git a/testdata/memcached-molecule-operator/config/rbac/role.yaml b/testdata/memcached-molecule-operator/config/rbac/role.yaml index 3289d44..d60645b 100644 --- a/testdata/memcached-molecule-operator/config/rbac/role.yaml +++ b/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 diff --git a/testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml b/testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml index 399bd4e..1a1e5f3 100644 --- a/testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml +++ b/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