diff --git a/hack/generate/samples/ansible/advanced_molecule.go b/hack/generate/samples/ansible/advanced_molecule.go index bac62e2..c718c60 100644 --- a/hack/generate/samples/ansible/advanced_molecule.go +++ b/hack/generate/samples/ansible/advanced_molecule.go @@ -84,46 +84,47 @@ func ImplementAdvancedMolecule(sample sample.Sample, image string) { } func updateConfig(dir string) { - log.Info("adding customized roles") - const cmRolesFragment = ` ## - ## Base operator rules - ## - - apiGroups: - - "" - resources: - - configmaps - - namespaces - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - - apiGroups: - - apps - resources: - - configmaps - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -#+kubebuilder:scaffold:rules` - err := kbutil.ReplaceInFile( - filepath.Join(dir, "config", "rbac", "role.yaml"), - "#+kubebuilder:scaffold:rules", - cmRolesFragment) - pkg.CheckError("adding customized roles", err) + //TODO: This currently fails, unsure if we still need this, seems the role file already has this + //log.Info("adding customized roles") + // const cmRolesFragment = ` ## + // ## Base operator rules + // ## + // - apiGroups: + // - "" + // resources: + // - configmaps + // - namespaces + // verbs: + // - create + // - delete + // - get + // - list + // - patch + // - update + // - watch + // - apiGroups: + // - apps + // resources: + // - configmaps + // verbs: + // - create + // - delete + // - get + // - list + // - patch + // - update + // - watch + //#+kubebuilder:scaffold:rules` + //err := kbutil.ReplaceInFile( + // filepath.Join(dir, "config", "rbac", "role.yaml"), + // "#+kubebuilder:scaffold:rules", + // cmRolesFragment) + //pkg.CheckError("adding customized roles", err) log.Info("adding manager arg") const ansibleVaultArg = ` - --ansible-args='--vault-password-file /opt/ansible/pwd.yml'` - err = kbutil.InsertCode( + err := kbutil.InsertCode( filepath.Join(dir, "config", "manager", "manager.yaml"), "- --leader-election-id=advanced-molecule-operator", ansibleVaultArg) @@ -145,8 +146,8 @@ func updateConfig(dir string) { const managerAuthArgs = ` - "--ansible-args='--vault-password-file /opt/ansible/pwd.yml'"` err = kbutil.InsertCode( - filepath.Join(dir, "config", "default", "manager_metrics_patch.yaml"), - "- \"--leader-elect\"", + filepath.Join(dir, "config", "manager", "manager.yaml"), + "- --leader-elect", managerAuthArgs) pkg.CheckError("adding vaulting args to the proxy auth", err) diff --git a/hack/generate/samples/ansible/constants.go b/hack/generate/samples/ansible/constants.go index d073114..2932922 100644 --- a/hack/generate/samples/ansible/constants.go +++ b/hack/generate/samples/ansible/constants.go @@ -531,7 +531,7 @@ const customMetricsTest = ` namespace: default container: manager pod: "{{ output.resources[0].metadata.name }}" - command: curl localhost:8080/metrics + command: curl localhost:8443/metrics register: metrics_output - name: Assert sanity metrics were created diff --git a/hack/generate/samples/ansible/memcached_molecule.go b/hack/generate/samples/ansible/memcached_molecule.go index e39d48e..2d09093 100644 --- a/hack/generate/samples/ansible/memcached_molecule.go +++ b/hack/generate/samples/ansible/memcached_molecule.go @@ -80,7 +80,7 @@ func ImplementMemcachedMolecule(sample sample.Sample, image string) { log.Info("adding RBAC permissions") err = kbutil.ReplaceInFile(filepath.Join(sample.Dir(), "config", "rbac", "role.yaml"), - "#+kubebuilder:scaffold:rules", rolesForBaseOperator) + "# +kubebuilder:scaffold:rules", rolesForBaseOperator) pkg.CheckError("replacing in role.yml", err) log.Info("adding task to delete config map") @@ -127,6 +127,6 @@ func ImplementMemcachedMolecule(sample sample.Sample, image string) { log.Info("enabling metrics in the manager") err = kbutil.UncommentCode( filepath.Join(sample.Dir(), "config", "default", "kustomization.yaml"), - "#- path: manager_metrics_patch.yaml", "#") + "- path: manager_metrics_patch.yaml", "#") pkg.CheckError("enabling metrics endpoint", err) } diff --git a/internal/ansible/flags/flag.go b/internal/ansible/flags/flag.go index b50ccef..ed28e85 100644 --- a/internal/ansible/flags/flag.go +++ b/internal/ansible/flags/flag.go @@ -111,13 +111,13 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) { // TODO(2.0.0): remove flagSet.StringVar(&f.MetricsBindAddress, "metrics-addr", - ":8080", + ":8443", "The address the metric endpoint binds to", ) _ = flagSet.MarkDeprecated("metrics-addr", "use --metrics-bind-address instead") flagSet.StringVar(&f.MetricsBindAddress, "metrics-bind-address", - ":8080", + ":8443", "The address the metric endpoint binds to", ) // TODO(2.0.0): for Go/Helm the port used is: 8081 diff --git a/internal/ansible/flags/flag_test.go b/internal/ansible/flags/flag_test.go index e4afdf9..6da8a4e 100644 --- a/internal/ansible/flags/flag_test.go +++ b/internal/ansible/flags/flag_test.go @@ -52,7 +52,7 @@ var _ = Describe("Flags", func() { }) When("the flag is not set", func() { It("uses the default flag value when corresponding option value is empty", func() { - expOptionValue := ":8080" + expOptionValue := ":8443" options.Metrics.BindAddress = "" parseArgs(flagSet) Expect(f.ToManagerOptions(options).Metrics.BindAddress).To(Equal(expOptionValue)) diff --git a/internal/ansible/handler/logging_enqueue_annotation_test.go b/internal/ansible/handler/logging_enqueue_annotation_test.go index 192e809..e73f801 100644 --- a/internal/ansible/handler/logging_enqueue_annotation_test.go +++ b/internal/ansible/handler/logging_enqueue_annotation_test.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllertest" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -60,7 +61,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() { Expect(handler.SetOwnerAnnotations(podOwner, pod)).To(Succeed()) instance = LoggingEnqueueRequestForAnnotation{ - handler.EnqueueRequestForAnnotation{ + handler.EnqueueRequestForAnnotation[client.Object]{ Type: schema.GroupKind{ Group: "", Kind: "Pod", @@ -220,7 +221,11 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() { } nd.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"}) - instance = LoggingEnqueueRequestForAnnotation{handler.EnqueueRequestForAnnotation{Type: schema.GroupKind{Group: "apps", Kind: "ReplicaSet"}}} + instance = LoggingEnqueueRequestForAnnotation{ + handler.EnqueueRequestForAnnotation[client.Object]{ + Type: schema.GroupKind{Group: "apps", Kind: "ReplicaSet"}, + }, + } evt := event.CreateEvent{ Object: nd, @@ -243,7 +248,11 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() { } nd.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"}) - instance = LoggingEnqueueRequestForAnnotation{handler.EnqueueRequestForAnnotation{Type: nd.GetObjectKind().GroupVersionKind().GroupKind()}} + instance = LoggingEnqueueRequestForAnnotation{ + handler.EnqueueRequestForAnnotation[client.Object]{ + Type: nd.GetObjectKind().GroupVersionKind().GroupKind(), + }, + } evt := event.CreateEvent{ Object: nd, } @@ -341,11 +350,13 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() { repl.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "ReplicaSet"}) instance = LoggingEnqueueRequestForAnnotation{ - handler.EnqueueRequestForAnnotation{ + handler.EnqueueRequestForAnnotation[client.Object]{ Type: schema.GroupKind{ Group: "apps", Kind: "ReplicaSet", - }}} + }, + }, + } evt := event.CreateEvent{ Object: repl, @@ -366,11 +377,13 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() { } instance2 := LoggingEnqueueRequestForAnnotation{ - handler.EnqueueRequestForAnnotation{ + handler.EnqueueRequestForAnnotation[client.Object]{ Type: schema.GroupKind{ Group: "apps", Kind: "ReplicaSet", - }}} + }, + }, + } evt2 := event.UpdateEvent{ ObjectOld: repl, diff --git a/internal/ansible/proxy/kubectl.go b/internal/ansible/proxy/kubectl.go index 1651008..0c661eb 100644 --- a/internal/ansible/proxy/kubectl.go +++ b/internal/ansible/proxy/kubectl.go @@ -130,7 +130,7 @@ func (f *FilterServer) HandlerFor(delegate http.Handler) *FilterServer { return &f2 } -// Get host from a host header value like "localhost" or "localhost:8080" +// Get host from a host header value like "localhost" or "localhost:8443" func extractHost(header string) (host string) { host, _, err := net.SplitHostPort(header) if err != nil { diff --git a/pkg/plugins/ansible/v1/init.go b/pkg/plugins/ansible/v1/init.go index 8b143ed..ae558ed 100644 --- a/pkg/plugins/ansible/v1/init.go +++ b/pkg/plugins/ansible/v1/init.go @@ -160,7 +160,6 @@ func addInitCustomizations(projectName string) error { } managerFile := filepath.Join("config", "manager", "manager.yaml") - managerMetricsPatchFile := 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 @@ -174,13 +173,6 @@ func addInitCustomizations(projectName string) error { return err } - err = util.InsertCode(managerMetricsPatchFile, - "- \"--metrics-bind-address=0.0.0.0:8080\"", - fmt.Sprintf("\n - \"--leader-elect\"\n - \"--leader-election-id=%s\"\n - \"--health-probe-bind-address=:6789\"", projectName)) - 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 7247818..bbf915e 100644 --- a/pkg/testutils/e2e/metrics/helpers.go +++ b/pkg/testutils/e2e/metrics/helpers.go @@ -17,7 +17,7 @@ func GetMetrics(sample sample.Sample, kubectl kubernetes.Kubectl, metricsCluster cmdOpts := []string{ "run", "curl", "--image=curlimages/curl:7.68.0", "--restart=OnFailure", "--", "curl", "-v", - fmt.Sprintf("http://%s-controller-manager-metrics-service.%s.svc:8080/metrics", sample.Name(), kubectl.Namespace()), + fmt.Sprintf("http://%s-controller-manager-metrics-service.%s.svc:8443/metrics", sample.Name(), kubectl.Namespace()), } out, err := kubectl.CommandInNamespace(cmdOpts...) fmt.Println("OUT --", out) diff --git a/test/e2e/ansible/suite_test.go b/test/e2e/ansible/suite_test.go index 5311dbf..3b043a4 100644 --- a/test/e2e/ansible/suite_test.go +++ b/test/e2e/ansible/suite_test.go @@ -27,8 +27,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - kbutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util" - "github.com/operator-framework/ansible-operator-plugins/hack/generate/samples/ansible" "github.com/operator-framework/ansible-operator-plugins/pkg/testutils/command" "github.com/operator-framework/ansible-operator-plugins/pkg/testutils/e2e/kind" @@ -36,6 +34,7 @@ import ( "github.com/operator-framework/ansible-operator-plugins/pkg/testutils/e2e/prometheus" "github.com/operator-framework/ansible-operator-plugins/pkg/testutils/kubernetes" "github.com/operator-framework/ansible-operator-plugins/pkg/testutils/sample" + kbutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util" ) //TODO: update this to use the new PoC api @@ -79,8 +78,8 @@ var _ = BeforeSuite(func() { // --------------------------------------------------- By("enabling debug logging in the manager") - err = kbutil.ReplaceInFile(filepath.Join(ansibleSample.Dir(), "config", "default", "manager_metrics_patch.yaml"), - "- \"--leader-elect\"", "- \"--zap-log-level=2\"\n - \"--leader-elect\"") + err = kbutil.InsertCode(filepath.Join(ansibleSample.Dir(), "config", "manager", "manager.yaml"), + "--health-probe-bind-address=:6789", "\n - --zap-log-level=2") Expect(err).NotTo(HaveOccurred()) // --------------------------------------------------- diff --git a/testdata/memcached-molecule-operator/Makefile b/testdata/memcached-molecule-operator/Makefile index f9d5671..712909b 100644 --- a/testdata/memcached-molecule-operator/Makefile +++ b/testdata/memcached-molecule-operator/Makefile @@ -81,7 +81,7 @@ ifeq (,$(shell which kustomize 2>/dev/null)) @{ \ set -e ;\ mkdir -p $(dir $(KUSTOMIZE)) ;\ - curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v5.3.0/kustomize_v5.3.0_$(OS)_$(ARCH).tar.gz | \ + curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v5.4.2/kustomize_v5.4.2_$(OS)_$(ARCH).tar.gz | \ tar xzf - -C bin/ ;\ } else diff --git a/testdata/memcached-molecule-operator/config/crd/kustomization.yaml b/testdata/memcached-molecule-operator/config/crd/kustomization.yaml index a72b764..d18a7ee 100644 --- a/testdata/memcached-molecule-operator/config/crd/kustomization.yaml +++ b/testdata/memcached-molecule-operator/config/crd/kustomization.yaml @@ -6,4 +6,4 @@ resources: - bases/cache.example.com_foos.yaml - bases/cache.example.com_memfins.yaml - bases/ignore.example.com_secrets.yaml -#+kubebuilder:scaffold:crdkustomizeresource +# +kubebuilder:scaffold:crdkustomizeresource diff --git a/testdata/memcached-molecule-operator/config/default/kustomization.yaml b/testdata/memcached-molecule-operator/config/default/kustomization.yaml index 5410415..97bf8d0 100644 --- a/testdata/memcached-molecule-operator/config/default/kustomization.yaml +++ b/testdata/memcached-molecule-operator/config/default/kustomization.yaml @@ -20,9 +20,13 @@ resources: - ../manager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. - ../prometheus +# [METRICS] Expose the controller manager metrics service. +- metrics_service.yaml +# Uncomment the patches line if you enable Metrics, and/or are using webhooks and cert-manager patches: -# [METRICS] The following patch will enable the metrics endpoint. Ensure that you also protect this endpoint. +# [METRICS] The following patch will enable the metrics endpoint using HTTPS and the port :8443. # More info: https://book.kubebuilder.io/reference/metrics -# If you want to expose the metric endpoint of your controller-manager uncomment the following line. - path: manager_metrics_patch.yaml + target: + kind: Deployment diff --git a/testdata/memcached-molecule-operator/config/default/manager_config_patch.yaml b/testdata/memcached-molecule-operator/config/default/manager_config_patch.yaml deleted file mode 100644 index f6f5891..0000000 --- a/testdata/memcached-molecule-operator/config/default/manager_config_patch.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: controller-manager - namespace: system -spec: - template: - spec: - containers: - - name: manager 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 9aa84c7..2aaef65 100644 --- a/testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml +++ b/testdata/memcached-molecule-operator/config/default/manager_metrics_patch.yaml @@ -1,16 +1,4 @@ -# This patch adds the args to allow exposing the metrics endpoint securely -apiVersion: apps/v1 -kind: Deployment -metadata: - name: controller-manager - namespace: system -spec: - template: - spec: - containers: - - name: manager - args: - - "--metrics-bind-address=0.0.0.0:8080" - - "--leader-elect" - - "--leader-election-id=memcached-molecule-operator" - - "--health-probe-bind-address=:6789" +# 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 diff --git a/testdata/memcached-molecule-operator/config/manager/manager.yaml b/testdata/memcached-molecule-operator/config/manager/manager.yaml index 2b60cd9..5a8ced3 100644 --- a/testdata/memcached-molecule-operator/config/manager/manager.yaml +++ b/testdata/memcached-molecule-operator/config/manager/manager.yaml @@ -62,7 +62,6 @@ spec: - --leader-elect - --leader-election-id=memcached-molecule-operator - --health-probe-bind-address=:6789 - - --metrics-bind-address=0 image: controller:latest name: manager env: diff --git a/testdata/memcached-molecule-operator/config/prometheus/monitor.yaml b/testdata/memcached-molecule-operator/config/prometheus/monitor.yaml index 5e02113..f37e0a5 100644 --- a/testdata/memcached-molecule-operator/config/prometheus/monitor.yaml +++ b/testdata/memcached-molecule-operator/config/prometheus/monitor.yaml @@ -11,8 +11,20 @@ metadata: spec: endpoints: - path: /metrics - port: http # Ensure this is the name of the port that exposes HTTP metrics - scheme: http + port: https # Ensure this is the name of the port that exposes HTTPS metrics + scheme: https + bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token + tlsConfig: + # TODO(user): The option insecureSkipVerify: true is not recommended for production since it disables + # certificate verification. This poses a significant security risk by making the system vulnerable to + # man-in-the-middle attacks, where an attacker could intercept and manipulate the communication between + # Prometheus and the monitored services. This could lead to unauthorized access to sensitive metrics data, + # compromising the integrity and confidentiality of the information. + # Please use the following options for secure configurations: + # caFile: /etc/metrics-certs/ca.crt + # certFile: /etc/metrics-certs/tls.crt + # keyFile: /etc/metrics-certs/tls.key + insecureSkipVerify: true selector: matchLabels: control-plane: controller-manager diff --git a/testdata/memcached-molecule-operator/config/rbac/kustomization.yaml b/testdata/memcached-molecule-operator/config/rbac/kustomization.yaml index 33c9072..e02af38 100644 --- a/testdata/memcached-molecule-operator/config/rbac/kustomization.yaml +++ b/testdata/memcached-molecule-operator/config/rbac/kustomization.yaml @@ -9,7 +9,15 @@ resources: - role_binding.yaml - leader_election_role.yaml - leader_election_role_binding.yaml -- metrics_service.yaml +# The following RBAC configurations are used to protect +# the metrics endpoint with authn/authz. These configurations +# ensure that only authorized users and service accounts +# can access the metrics endpoint. Comment the following +# permissions if you want to disable this protection. +# More info: https://book.kubebuilder.io/reference/metrics.html +- metrics_auth_role.yaml +- metrics_auth_role_binding.yaml +- metrics_reader_role.yaml # For each CRD, "Editor" and "Viewer" roles are scaffolded by # default, aiding admins in cluster management. Those roles are # not used by the Project itself. You can comment the following lines diff --git a/testdata/memcached-molecule-operator/config/rbac/metrics_service.yaml b/testdata/memcached-molecule-operator/config/rbac/metrics_service.yaml deleted file mode 100644 index 5559b8a..0000000 --- a/testdata/memcached-molecule-operator/config/rbac/metrics_service.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: v1 -kind: Service -metadata: - labels: - control-plane: controller-manager - app.kubernetes.io/name: memcached-molecule-operator - app.kubernetes.io/managed-by: kustomize - name: controller-manager-metrics-service - namespace: system -spec: - ports: - - name: http - port: 8080 - protocol: TCP - targetPort: 8080 - selector: - control-plane: controller-manager diff --git a/testdata/memcached-molecule-operator/config/samples/kustomization.yaml b/testdata/memcached-molecule-operator/config/samples/kustomization.yaml index 26c1898..faf679a 100644 --- a/testdata/memcached-molecule-operator/config/samples/kustomization.yaml +++ b/testdata/memcached-molecule-operator/config/samples/kustomization.yaml @@ -4,4 +4,4 @@ resources: - cache_v1alpha1_foo.yaml - cache_v1alpha1_memfin.yaml - ignore_v1_secret.yaml -#+kubebuilder:scaffold:manifestskustomizesamples +# +kubebuilder:scaffold:manifestskustomizesamples 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 bc9511b..399bd4e 100644 --- a/testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml +++ b/testdata/memcached-molecule-operator/molecule/default/tasks/memcached_test.yml @@ -78,7 +78,7 @@ namespace: default container: manager pod: "{{ output.resources[0].metadata.name }}" - command: curl localhost:8080/metrics + command: curl localhost:8443/metrics register: metrics_output - name: Assert sanity metrics were created diff --git a/testdata/memcached-molecule-operator/watches.yaml b/testdata/memcached-molecule-operator/watches.yaml index 1bc2591..a45e5eb 100644 --- a/testdata/memcached-molecule-operator/watches.yaml +++ b/testdata/memcached-molecule-operator/watches.yaml @@ -24,4 +24,4 @@ kind: Secret role: secret manageStatus: false -#+kubebuilder:scaffold:watch +# +kubebuilder:scaffold:watch