feat: expose chart-values api without rbac-proxy

Signed-off-by: Ilya Drey <[email protected]>

Author: drey

images/chart-values-controller/cmd/chart-values-controller/main.go

@@ -55,6 +55,8 @@ func main() {
 		metricsAddr     string
 		healthProbeAddr string
 		apiAddr         string
+		apiTLSCertFile  string
+		apiTLSKeyFile   string
 		cacheDir        string
 		cacheTTL        time.Duration
 		sourceInterval  time.Duration
@@ -64,6 +66,8 @@ func main() {
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metrics endpoint binds to.")
 	flag.StringVar(&healthProbeAddr, "health-probe-bind-address", ":9440", "The address the health probe endpoint binds to.")
 	flag.StringVar(&apiAddr, "api-bind-address", "127.0.0.1:8081", "The address the chart-values HTTP API binds to.")
+	flag.StringVar(&apiTLSCertFile, "api-tls-cert-file", "", "Path to the PEM-encoded certificate for the chart-values HTTP API; enables TLS together with --api-tls-key-file.")
+	flag.StringVar(&apiTLSKeyFile, "api-tls-key-file", "", "Path to the PEM-encoded private key for the chart-values HTTP API; enables TLS together with --api-tls-cert-file.")
 	flag.StringVar(&cacheDir, "cache-dir", "/cache", "Directory used to cache extracted values.yaml files.")
 	flag.DurationVar(&cacheTTL, "chart-values-cache-ttl", time.Hour, "Lifetime of auxiliary source resources and their cache entries; refreshed on each request.")
 	flag.DurationVar(&sourceInterval, "source-interval", 10*time.Minute, "Reconcile interval set on auxiliary source resources.")
@@ -81,6 +85,11 @@ func main() {
 	}
 	maxArtifactBytes := int64(maxChartSizeMB) << 20
 
+	if (apiTLSCertFile == "") != (apiTLSKeyFile == "") {
+		logger.Error(nil, "--api-tls-cert-file and --api-tls-key-file must be set together")
+		os.Exit(1)
+	}
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme: scheme,
 		Metrics: metricsserver.Options{
@@ -107,7 +116,11 @@ func main() {
 		maxArtifactBytes,
 	)
 
-	if err := mgr.Add(server.New(apiAddr, res)); err != nil {
+	apiServer := server.New(apiAddr, res, server.NewOptions{
+		TLSCertFile: apiTLSCertFile,
+		TLSKeyFile:  apiTLSKeyFile,
+	})
+	if err := mgr.Add(apiServer); err != nil {
 		logger.Error(err, "unable to add HTTP server")
 		os.Exit(1)
 	}

images/chart-values-controller/internal/server/server.go

@@ -40,12 +40,26 @@ type chartValuesResolver interface {
 // Server exposes the chart-values HTTP API as a controller-runtime Runnable.
 // It does not require leader election so it can serve from any replica.
 type Server struct {
-	addr     string
-	resolver chartValuesResolver
+	addr        string
+	resolver    chartValuesResolver
+	tlsCertFile string
+	tlsKeyFile  string
 }
 
-func New(addr string, res chartValuesResolver) *Server {
-	return &Server{addr: addr, resolver: res}
+// NewOptions carries optional server configuration. When both TLSCertFile and
+// TLSKeyFile are set the API is served over TLS.
+type NewOptions struct {
+	TLSCertFile string
+	TLSKeyFile  string
+}
+
+func New(addr string, res chartValuesResolver, opts NewOptions) *Server {
+	return &Server{
+		addr:        addr,
+		resolver:    res,
+		tlsCertFile: opts.TLSCertFile,
+		tlsKeyFile:  opts.TLSKeyFile,
+	}
 }
 
 // NeedLeaderElection reports that the HTTP server runs on every replica.
@@ -72,7 +86,14 @@ func (s *Server) Start(ctx context.Context) error {
 		_ = srv.Shutdown(shutdownCtx) //nolint:contextcheck // parent ctx is done, a fresh one is required for graceful shutdown
 	}()
 
-	if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
+	serve := srv.ListenAndServe
+	if s.tlsCertFile != "" && s.tlsKeyFile != "" {
+		serve = func() error {
+			return srv.ListenAndServeTLS(s.tlsCertFile, s.tlsKeyFile)
+		}
+	}
+
+	if err := serve(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		return err
 	}
 

images/chart-values-controller/internal/server/server_test.go

@@ -39,7 +39,7 @@ func (f fakeResolver) Resolve(_ context.Context, _ resolver.Request) (resolver.R
 func do(t *testing.T, res chartValuesResolver, body string) *httptest.ResponseRecorder {
 	t.Helper()
 
-	srv := New("", res)
+	srv := New("", res, NewOptions{})
 	rec := httptest.NewRecorder()
 	req := httptest.NewRequest(http.MethodPost, "/v1/chart-values", strings.NewReader(body))
 	srv.handleChartValues(rec, req)
@@ -96,7 +96,7 @@ func TestHandleOutcomeStatusCodes(t *testing.T) {
 		if rec.Code != c.wantStatus {
 			t.Fatalf("outcome %s: status = %d, want %d", c.outcome, rec.Code, c.wantStatus)
 		}
-		assertErrorCode(t, rec.Body.Bytes(), c.wantCode)
+		assertCode(t, rec.Body.Bytes(), "code", c.wantCode)
 	}
 }
 
@@ -105,39 +105,23 @@ func TestHandleInvalidJSON(t *testing.T) {
 	if rec.Code != http.StatusBadRequest {
 		t.Fatalf("status = %d, want 400", rec.Code)
 	}
-	assertErrorCode(t, rec.Body.Bytes(), "INVALID_REQUEST")
+	assertCode(t, rec.Body.Bytes(), "code", "INVALID_REQUEST")
 }
 
 func TestHandleMissingFields(t *testing.T) {
 	rec := do(t, fakeResolver{}, `{"repositoryName":"github","chart":"podinfo"}`)
 	if rec.Code != http.StatusBadRequest {
 		t.Fatalf("status = %d, want 400", rec.Code)
 	}
-	assertErrorCode(t, rec.Body.Bytes(), "INVALID_REQUEST")
+	assertCode(t, rec.Body.Bytes(), "code", "INVALID_REQUEST")
 }
 
 func TestHandleInternalError(t *testing.T) {
 	rec := do(t, fakeResolver{err: context.DeadlineExceeded}, validBody)
 	if rec.Code != http.StatusInternalServerError {
 		t.Fatalf("status = %d, want 500", rec.Code)
 	}
-	assertErrorCode(t, rec.Body.Bytes(), "INTERNAL")
-}
-
-func assertErrorCode(t *testing.T, body []byte, want string) {
-	t.Helper()
-
-	var resp struct {
-		Error struct {
-			Code string `json:"code"`
-		} `json:"error"`
-	}
-	if err := json.Unmarshal(body, &resp); err != nil {
-		t.Fatalf("decode: %v", err)
-	}
-	if resp.Error.Code != want {
-		t.Fatalf("error code = %q, want %q", resp.Error.Code, want)
-	}
+	assertCode(t, rec.Body.Bytes(), "code", "INTERNAL")
 }
 
 func assertCode(t *testing.T, body []byte, field, want string) {

images/hooks/pkg/hooks/tls-certificates-controller/hook.go

@@ -38,3 +38,20 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS
 	FullValuesPathPrefix: fmt.Sprintf("%s.internal.controller.cert", settings.ModuleName),
 	CommonCAValuesPath:   fmt.Sprintf("%s.internal.rootCA", settings.ModuleName),
 })
+
+var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLSHookConf{
+	CN:            settings.ChartValuesControllerCertCN,
+	TLSSecretName: "chart-values-controller-tls",
+	Namespace:     settings.ModuleNamespace,
+	SANs: tlscertificate.DefaultSANs([]string{
+		"localhost",
+		"127.0.0.1",
+		settings.ChartValuesControllerCertCN,
+		settings.ChartValuesAPIServiceName,
+		fmt.Sprintf("%s.%s", settings.ChartValuesAPIServiceName, settings.ModuleNamespace),
+		fmt.Sprintf("%s.%s.svc", settings.ChartValuesAPIServiceName, settings.ModuleNamespace),
+	}),
+
+	FullValuesPathPrefix: fmt.Sprintf("%s.internal.chartValuesController.cert", settings.ModuleName),
+	CommonCAValuesPath:   fmt.Sprintf("%s.internal.rootCA", settings.ModuleName),
+})

images/hooks/pkg/settings/certificate.go

@@ -19,4 +19,5 @@ package settings
 const (
 	ControllerCertCN            string = "operator-helm-controller"
 	ChartValuesControllerCertCN string = "chart-values-controller"
+	ChartValuesAPIServiceName   string = "chart-values-api"
 )

templates/chart-values-controller/deployment.yaml

@@ -76,19 +76,24 @@ spec:
           args:
             - --metrics-bind-address=:8080
             - --health-probe-bind-address=:9440
-            - --api-bind-address=127.0.0.1:8081
+            - --api-bind-address=:9443
+            - --api-tls-cert-file=/etc/chart-values-controller/tls/tls.crt
+            - --api-tls-key-file=/etc/chart-values-controller/tls/tls.key
             - --cache-dir=/cache
             - --chart-values-cache-ttl=1h
             - --max-chart-size-mb=100
           volumeMounts:
             - mountPath: /cache
               name: cache
+            - mountPath: /etc/chart-values-controller/tls
+              name: api-tls
+              readOnly: true
             {{- include "kube_api_rewriter.kubeconfig_volume_mount" . | nindent 12 }}
           ports:
             - containerPort: 8080
               name: metrics
               protocol: TCP
-            - containerPort: 8081 
+            - containerPort: 9443
               name: api
               protocol: TCP
             - containerPort: 9440
@@ -121,7 +126,6 @@ spec:
         {{- $_ := set $kubeRbacProxySettings "portName" "rbac-proxy" }}
         {{- $_ := set $kubeRbacProxySettings "upstreams" (list
            (dict "upstream" "http://127.0.0.1:8080/metrics" "path" "/metrics" "name" "chart-values-controller")
-           (dict "upstream" "http://127.0.0.1:8081/v1/chart-values" "path" "/v1/chart-values" "name" "chart-values-controller" "subresource" "api")
            (dict "upstream" "http://127.0.0.1:9090/metrics" "path" "/proxy/metrics" "name" "kube-api-rewriter")
            (dict "upstream" "http://127.0.0.1:9090/healthz" "path" "/proxy/healthz" "name" "kube-api-rewriter")
            (dict "upstream" "http://127.0.0.1:9090/readyz" "path" "/proxy/readyz" "name" "kube-api-rewriter")
@@ -136,4 +140,7 @@ spec:
       volumes:
         - name: cache
           emptyDir: {}
+        - name: api-tls
+          secret:
+            secretName: chart-values-controller-tls
         {{- include "kube_api_rewriter.kubeconfig_volume" . | nindent 8 }}

templates/chart-values-controller/secret-tls.yaml

@@ -0,0 +1,12 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: chart-values-controller-tls
+  namespace: d8-{{ .Chart.Name }}
+  {{- include "helm_lib_module_labels" (list . (dict "app" "chart-values-controller")) | nindent 2 }}
+type: kubernetes.io/tls
+data:
+  ca.crt: {{ .Values.operatorHelm.internal.chartValuesController.cert.ca | b64enc }}
+  tls.crt: {{ .Values.operatorHelm.internal.chartValuesController.cert.crt | b64enc }}
+  tls.key: {{ .Values.operatorHelm.internal.chartValuesController.cert.key | b64enc }}

templates/chart-values-controller/service.yaml

@@ -7,9 +7,9 @@ metadata:
   {{- include "helm_lib_module_labels" (list . (dict "app" "chart-values-controller")) | nindent 2 }}
 spec:
   ports:
-    - name: rbac-proxy
+    - name: api
       port: 443
-      targetPort: rbac-proxy
+      targetPort: api
       protocol: TCP
   selector:
     app: chart-values-controller

templates/rbac-to-us.yaml

@@ -29,32 +29,3 @@ subjects:
   name: prometheus
   namespace: d8-monitoring
 {{- end }}
-{{- if (.Values.global.enabledModules | has "console") }}
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: Role
-metadata:
-  name: console-access-to-operator-helm
-  namespace: d8-{{ .Chart.Name }}
-  {{- include "helm_lib_module_labels" (list .) | nindent 2 }}
-rules:
-- apiGroups: ["apps"]
-  resources: ["deployments/api"]
-  resourceNames: ["chart-values-controller"]
-  verbs: ["create"]
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: RoleBinding
-metadata:
-  name: console-access-to-operator-helm
-  namespace: d8-{{ .Chart.Name }}
-  {{- include "helm_lib_module_labels" (list .) | nindent 2 }}
-roleRef:
-  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: console-access-to-operator-helm
-subjects:
-- kind: ServiceAccount
-  name: backend
-  namespace: d8-console
-{{- end }}