Skip to content

Commit 3ad1b57

Browse files
authored
adding metrics-require-rbac flag, metrics-secure validation logic and corresponding scaffolding logic (#116)
Signed-off-by: Adam D. Cornett <[email protected]>
1 parent 3aaf111 commit 3ad1b57

File tree

11 files changed

+144
-9
lines changed

11 files changed

+144
-9
lines changed

go.mod

+15
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,15 @@ require (
3636
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect
3737
github.com/beorn7/perks v1.0.1 // indirect
3838
github.com/blang/semver/v4 v4.0.0 // indirect
39+
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
3940
github.com/cespare/xxhash/v2 v2.3.0 // indirect
4041
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
4142
github.com/emicklei/go-restful/v3 v3.11.2 // indirect
4243
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
4344
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
45+
github.com/felixge/httpsnoop v1.0.4 // indirect
4446
github.com/fsnotify/fsnotify v1.7.0 // indirect
47+
github.com/go-logr/stdr v1.2.2 // indirect
4548
github.com/go-logr/zapr v1.3.0 // indirect
4649
github.com/go-openapi/jsonpointer v0.20.2 // indirect
4750
github.com/go-openapi/jsonreference v0.20.4 // indirect
@@ -57,6 +60,7 @@ require (
5760
github.com/google/gofuzz v1.2.0 // indirect
5861
github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5 // indirect
5962
github.com/google/uuid v1.6.0 // indirect
63+
github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect
6064
github.com/h2non/filetype v1.1.3 // indirect
6165
github.com/h2non/go-is-svg v0.0.0-20160927212452-35e8c4b0612c // indirect
6266
github.com/hashicorp/hcl v1.0.0 // indirect
@@ -84,6 +88,14 @@ require (
8488
github.com/spf13/cast v1.6.0 // indirect
8589
github.com/stoewer/go-strcase v1.3.0 // indirect
8690
github.com/subosito/gotenv v1.6.0 // indirect
91+
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect
92+
go.opentelemetry.io/otel v1.24.0 // indirect
93+
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.23.1 // indirect
94+
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.23.1 // indirect
95+
go.opentelemetry.io/otel/metric v1.24.0 // indirect
96+
go.opentelemetry.io/otel/sdk v1.23.1 // indirect
97+
go.opentelemetry.io/otel/trace v1.24.0 // indirect
98+
go.opentelemetry.io/proto/otlp v1.1.0 // indirect
8799
go.uber.org/multierr v1.11.0 // indirect
88100
go.uber.org/zap v1.26.0 // indirect
89101
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
@@ -104,8 +116,11 @@ require (
104116
gopkg.in/ini.v1 v1.67.0 // indirect
105117
gopkg.in/yaml.v2 v2.4.0 // indirect
106118
gopkg.in/yaml.v3 v3.0.1 // indirect
119+
k8s.io/apiserver v0.30.5 // indirect
120+
k8s.io/component-base v0.30.5 // indirect
107121
k8s.io/klog/v2 v2.120.1 // indirect
108122
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
123+
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect
109124
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
110125
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
111126
)

go.sum

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

hack/generate/samples/ansible/constants.go

+15-2
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,17 @@ const rolesForBaseOperator = `
516516
- patch
517517
- update
518518
- watch
519+
##
520+
## Apply metrics_reader role for testing
521+
## This should align with metrics_reader.yaml
522+
## scaffolded by kubebuilder, but by default
523+
## that role is not applied to the generated
524+
## controller, so we apply it here.
525+
##
526+
- nonResourceURLs:
527+
- "/metrics"
528+
verbs:
529+
- get
519530
#+kubebuilder:scaffold:rules
520531
`
521532

@@ -526,12 +537,14 @@ const customMetricsTest = `
526537
label_selectors:
527538
- "control-plane = controller-manager"
528539
register: output
540+
529541
- name: Curl the metrics from the manager
530542
kubernetes.core.k8s_exec:
531-
namespace: default
543+
namespace: "{{ output.resources[0].metadata.namespace }}"
532544
container: manager
533545
pod: "{{ output.resources[0].metadata.name }}"
534-
command: curl localhost:8443/metrics
546+
command: >
547+
bash -c 'curl -k -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://localhost:8443/metrics'
535548
register: metrics_output
536549
537550
- name: Assert sanity metrics were created

internal/ansible/flags/flag.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/spf13/pflag"
2323
"k8s.io/client-go/tools/leaderelection/resourcelock"
2424
"sigs.k8s.io/controller-runtime/pkg/manager"
25+
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
2526
"sigs.k8s.io/controller-runtime/pkg/webhook"
2627
)
2728

@@ -48,6 +49,7 @@ type Flags struct {
4849
ProxyPort int
4950
EnableHTTP2 bool
5051
SecureMetrics bool
52+
MetricsRequireRBAC bool
5153

5254
// If not nil, used to deduce which flags were set in the CLI.
5355
flagSet *pflag.FlagSet
@@ -194,12 +196,16 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
194196
false,
195197
"enables HTTP/2 on the webhook and metrics servers",
196198
)
197-
198199
flagSet.BoolVar(&f.SecureMetrics,
199200
"metrics-secure",
200201
false,
201202
"enables secure serving of the metrics endpoint",
202203
)
204+
flagSet.BoolVar(&f.MetricsRequireRBAC,
205+
"metrics-require-rbac",
206+
false,
207+
"enables protection of the metrics endpoint with RBAC-based authn/authz."+
208+
"see https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/filters#WithAuthenticationAndAuthorization for more info")
203209
}
204210

205211
// ToManagerOptions uses the flag set in f to configure options.
@@ -254,5 +260,14 @@ func (f *Flags) ToManagerOptions(options manager.Options) manager.Options {
254260
options.Metrics.TLSOpts = append(options.Metrics.TLSOpts, disableHTTP2)
255261
}
256262
options.Metrics.SecureServing = f.SecureMetrics
263+
264+
if f.MetricsRequireRBAC {
265+
// FilterProvider is used to protect the metrics endpoint with authn/authz.
266+
// These configurations ensure that only authorized users and service accounts
267+
// can access the metrics endpoint. The RBAC are configured in 'config/rbac/kustomization.yaml'. More info:
268+
// https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/filters#WithAuthenticationAndAuthorization
269+
options.Metrics.FilterProvider = filters.WithAuthenticationAndAuthorization
270+
}
271+
257272
return options
258273
}

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

+6
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ func NewCmd() *cobra.Command {
7575
cmd := &cobra.Command{
7676
Use: "run",
7777
Short: "Run the operator",
78+
Args: func(cmd *cobra.Command, args []string) error {
79+
if cmd.Flag("metrics-require-rbac").Value.String() == "true" && cmd.Flag("metrics-secure").Value.String() == "false" {
80+
return errors.New("--metrics-secure flag is required when --metrics-require-rbac is present")
81+
}
82+
return nil
83+
},
7884
Run: func(cmd *cobra.Command, _ []string) {
7985
logf.SetLogger(zapf.New(zapf.UseFlagOptions(opts)))
8086
run(cmd, f)

pkg/plugins/ansible/v1/init.go

+22
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ func addInitCustomizations(projectName string) error {
160160
}
161161

162162
managerFile := filepath.Join("config", "manager", "manager.yaml")
163+
managerMetricsPatch := filepath.Join("config", "default", "manager_metrics_patch.yaml")
163164

164165
// todo: we ought to use afero instead. Replace this methods to insert/update
165166
// by https://github.com/kubernetes-sigs/kubebuilder/pull/2119
@@ -173,6 +174,27 @@ func addInitCustomizations(projectName string) error {
173174
return err
174175
}
175176

177+
// Enable the proper auth/metrics flags
178+
err = util.ReplaceInFile(managerMetricsPatch,
179+
`# This patch adds the args to allow exposing the metrics endpoint using HTTPS
180+
- op: add
181+
path: /spec/template/spec/containers/0/args/0
182+
value: --metrics-bind-address=:8443`, `# This patch adds the args to allow exposing the metrics endpoint using HTTPS
183+
- op: add
184+
path: /spec/template/spec/containers/0/args/0
185+
value: --metrics-bind-address=:8443
186+
# This patch adds the args to allow securing the metrics endpoint
187+
- op: add
188+
path: /spec/template/spec/containers/0/args/0
189+
value: --metrics-secure
190+
# This patch adds the args to allow RBAC-based authn/authz the metrics endpoint
191+
- op: add
192+
path: /spec/template/spec/containers/0/args/0
193+
value: --metrics-require-rbac`)
194+
if err != nil {
195+
return err
196+
}
197+
176198
// update default resource request and limits with bigger values
177199
const resourcesLimitsFragment = ` resources:
178200
limits:

pkg/testutils/e2e/metrics/helpers.go

+43-3
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,60 @@
11
package metrics
22

33
import (
4+
"encoding/base64"
45
"fmt"
6+
"strings"
57
"time"
68

79
"github.com/onsi/ginkgo/v2"
810
"github.com/onsi/gomega"
911

1012
"github.com/operator-framework/ansible-operator-plugins/pkg/testutils/kubernetes"
1113
"github.com/operator-framework/ansible-operator-plugins/pkg/testutils/sample"
14+
"github.com/operator-framework/ansible-operator-plugins/test/common"
1215
)
1316

1417
// GetMetrics creates a pod with the permissions to `curl` metrics. It will then return the output of the `curl` pod
1518
func GetMetrics(sample sample.Sample, kubectl kubernetes.Kubectl, metricsClusterRoleBindingName string) string {
19+
ginkgo.By("granting permissions to access the metrics and read the token")
20+
out, err := kubectl.Command("create", "clusterrolebinding", metricsClusterRoleBindingName,
21+
fmt.Sprintf("--clusterrole=%s-metrics-reader", sample.Name()),
22+
fmt.Sprintf("--serviceaccount=%s:%s", kubectl.Namespace(), kubectl.ServiceAccount()))
23+
fmt.Println("OUT --", out)
24+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
25+
26+
// As of Kubernetes 1.24 a ServiceAccount no longer has a ServiceAccount token secret autogenerated. We have to create it manually here
27+
ginkgo.By("Creating the ServiceAccount token")
28+
secretFile, err := common.GetSASecret(kubectl.ServiceAccount(), sample.Dir())
29+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
30+
gomega.Eventually(func() error {
31+
out, err = kubectl.Apply(true, "-f", secretFile)
32+
fmt.Println("OUT -- ", out)
33+
return err
34+
}, time.Minute, time.Second).Should(gomega.Succeed())
35+
36+
ginkgo.By("reading the metrics token")
37+
// Filter token query by service account in case more than one exists in a namespace.
38+
query := fmt.Sprintf(`{.items[?(@.metadata.annotations.kubernetes\.io/service-account\.name=="%s")].data.token}`,
39+
kubectl.ServiceAccount(),
40+
)
41+
out, err = kubectl.Get(true, "secrets")
42+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
43+
fmt.Println("OUT --", out)
44+
b64Token, err := kubectl.Get(true, "secrets", "-o=jsonpath="+query)
45+
fmt.Println("OUT--", b64Token)
46+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
47+
token, err := base64.StdEncoding.DecodeString(strings.TrimSpace(b64Token))
48+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
49+
gomega.Expect(len(token)).To(gomega.BeNumerically(">", 0))
50+
1651
ginkgo.By("creating a curl pod")
1752
cmdOpts := []string{
1853
"run", "curl", "--image=curlimages/curl:7.68.0", "--restart=OnFailure", "--",
19-
"curl", "-v",
20-
fmt.Sprintf("http://%s-controller-manager-metrics-service.%s.svc:8443/metrics", sample.Name(), kubectl.Namespace()),
54+
"curl", "-v", "-k", "-H", fmt.Sprintf(`Authorization: Bearer %s`, token),
55+
fmt.Sprintf("https://%s-controller-manager-metrics-service.%s.svc:8443/metrics", sample.Name(), kubectl.Namespace()),
2156
}
22-
out, err := kubectl.CommandInNamespace(cmdOpts...)
57+
out, err = kubectl.CommandInNamespace(cmdOpts...)
2358
fmt.Println("OUT --", out)
2459
gomega.Expect(err).NotTo(gomega.HaveOccurred())
2560

@@ -58,5 +93,10 @@ func CleanUpMetrics(kubectl kubernetes.Kubectl, metricsClusterRoleBindingName st
5893
return fmt.Errorf("encountered an error when deleting the metrics pod: %w", err)
5994
}
6095

96+
_, err = kubectl.Delete(false, "clusterrolebinding", metricsClusterRoleBindingName)
97+
if err != nil {
98+
return fmt.Errorf("encountered an error when deleting the metrics clusterrolebinding: %w", err)
99+
}
100+
61101
return nil
62102
}

test/e2e/ansible/cluster_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ var _ = Describe("Running ansible projects", func() {
5757
})
5858

5959
AfterEach(func() {
60+
By("deleting curl pod")
61+
testutils.WrapWarnOutput(kctl.Delete(false, "pod", "curl"))
62+
6063
By("cleaning up metrics")
6164
Expect(metrics.CleanUpMetrics(kctl, metricsClusterRoleBindingName)).To(Succeed())
6265

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

+8
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,11 @@
22
- op: add
33
path: /spec/template/spec/containers/0/args/0
44
value: --metrics-bind-address=:8443
5+
# This patch adds the args to allow securing the metrics endpoint
6+
- op: add
7+
path: /spec/template/spec/containers/0/args/0
8+
value: --metrics-secure
9+
# This patch adds the args to allow RBAC-based authn/authz the metrics endpoint
10+
- op: add
11+
path: /spec/template/spec/containers/0/args/0
12+
value: --metrics-require-rbac

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

+11
Original file line numberDiff line numberDiff line change
@@ -122,5 +122,16 @@ rules:
122122
- patch
123123
- update
124124
- watch
125+
##
126+
## Apply metrics_reader role for testing
127+
## This should align with metrics_reader.yaml
128+
## scaffolded by kubebuilder, but by default
129+
## that role is not applied to the generated
130+
## controller, so we apply it here.
131+
##
132+
- nonResourceURLs:
133+
- "/metrics"
134+
verbs:
135+
- get
125136
#+kubebuilder:scaffold:rules
126137

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,14 @@
7373
label_selectors:
7474
- "control-plane = controller-manager"
7575
register: output
76+
7677
- name: Curl the metrics from the manager
7778
kubernetes.core.k8s_exec:
78-
namespace: default
79+
namespace: "{{ output.resources[0].metadata.namespace }}"
7980
container: manager
8081
pod: "{{ output.resources[0].metadata.name }}"
81-
command: curl localhost:8443/metrics
82+
command: >
83+
bash -c 'curl -k -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://localhost:8443/metrics'
8284
register: metrics_output
8385

8486
- name: Assert sanity metrics were created

0 commit comments

Comments
 (0)