From f88e66c1f500df61a9de1279931bdd5d02ce60c9 Mon Sep 17 00:00:00 2001 From: Evan Nemerson Date: Tue, 28 Apr 2026 11:06:49 -0400 Subject: [PATCH] CP-28161: Remove API key from webhook server The webhook server (cloudzero-webhook binary; previously known as the insights-controller) used to push metrics directly to the CloudZero cloud API and authenticated each request with the customer's API key. It now always pushes to the in-cluster aggregator, which is unauthenticated on the cluster-internal hop. The API key secret mount, the Authorization: Bearer header, the in-process secret monitor that polled the key file for rotation, and the legacy query-string encoding of cluster_name/cloud_account_id/region in the destination URL all became dead weight on this code path. Functional Change: Before: The webhook pod mounted the cloudzero-api-key secret, the rendered server-config.yaml included api_key_path:, and the pusher posted to http://aggregator/collector?cluster_name=...&cloud_account_id= ...®ion=... with an `Authorization: Bearer $KEY` header. A secret monitor polled the API key file every 10 seconds for in-process rotation. After: The webhook pod has no api-key volume or volumeMount, the rendered server-config.yaml has no api_key_path field, and the pusher posts to the bare collector URL with no Authorization header. The secret monitor is no longer wired into the webhook entry point. The aggregator (collector and shipper), the agent (Prometheus and validator init containers), the config-loader Job, and other components that legitimately still need the key are unchanged. Solution: 1. helm/templates/_cm_helpers.tpl: removed the api_key_path: line from the cloudzero-agent.insightsController.configuration helper. The aggregator's configuration helper still emits api_key_path because the shipper continues to need the key for uploads. 2. helm/templates/webhook-deploy.yaml: removed the cloudzero-api-key volumeMount and volume blocks (both gated on existingSecretName or apiKey). 3. app/config/webhook/settings.go: removed the unexported RemoteWrite.apiKey field, the GetAPIKey/SetAPIKey methods, the absFilePath and isValidURL helpers, and the Settings.mu mutex (it only protected the API-key read/write pair). Simplified setRemoteWriteURL() to assign Destination verbatim with a single URL validation; the aggregator's /collector handler does not parse cluster_name/cloud_account_id/region query parameters (only the shipper consumes those, on its own outbound path). The APIKeyPath field is kept as a deprecated tombstone so legacy server-config.yaml files and pod specs that still set API_KEY_PATH parse cleanly even under future strict-mode YAML/env decoders. 4. app/domain/pusher/pusher.go: removed the GetAPIKey lookup, the empty-key error branch, the Authorization header, and the apiKey parameter from pushMetrics. 5. app/functions/webhook/main.go: removed the monitor.NewSecretMonitor instantiation and its Run/Shutdown wiring. The monitor package is still imported for TLS reloading on SIGHUP, which is unrelated. Also updated the startup log message from "Starting CloudZero Insights Controller" to "Starting CloudZero Webhook Server" to use the current component name. 6. app/domain/monitor/secret_monitor_test.go: replaced the test's dependency on the webhook config.Settings (which no longer implements MonitoredAPIKey) with a small in-test file-backed fake. 7. app/config/webhook/settings_test.go and app/domain/pusher/pusher_test.go: removed API-key fixtures; the pusher test now positively asserts that no Authorization header is sent. 8. helm/tests/webhook_no_apikey_mount_test.yaml: new helm-unittest suite asserting the rendered webhook Deployment has no cloudzero-api-key volume or volumeMount, and the rendered webhook ConfigMap contains no api_key_path field and no cluster_name/cloud_account_id query string fragments. Validation: - Go unit tests pass for app/config/webhook, app/domain/pusher, and app/domain/monitor. - make lint-go is clean; make helm-lint is clean; make helm-test passes, including the new helm-unittest suite. - Generated chart manifests under tests/helm/template/ show only the intended deletions across all six variants (alloy, cert-manager, federated, istio, kubestate, manifest): one api_key_path: line in the webhook ConfigMap, one four-line volumeMount block, and one three-line volume block per variant. No other resources change. - KIND end-to-end: deployed the freshly built image, confirmed the webhook pod has no cloudzero-api-key volume or volumeMount, the rendered server-config.yaml has no api_key_path, and the destination URL has no query string. Exercised the admission flow with test pods and confirmed the pusher's "Pushing records to remote write endpoint" log fires with records flowing to the in-cluster aggregator. - Development EKS cluster end-to-end: helm upgrade rolled cleanly, webhook started with no API-key plumbing and no startup errors, admission requests exercised, "Pushing records to remote write endpoint" log confirmed the full data path through the new pusher code. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/config/webhook/settings.go | 112 ++++--------------- app/config/webhook/settings_test.go | 28 +---- app/domain/monitor/secret_monitor_test.go | 32 +++++- app/domain/pusher/pusher.go | 11 +- app/domain/pusher/pusher_test.go | 48 ++------ app/functions/webhook/main.go | 15 +-- helm/templates/_cm_helpers.tpl | 1 - helm/templates/webhook-deploy.yaml | 11 -- helm/tests/webhook_no_apikey_mount_test.yaml | 70 ++++++++++++ tests/helm/template/alloy.yaml | 8 -- tests/helm/template/cert-manager.yaml | 8 -- tests/helm/template/federated.yaml | 8 -- tests/helm/template/istio.yaml | 8 -- tests/helm/template/kubestate.yaml | 8 -- tests/helm/template/manifest.yaml | 8 -- 15 files changed, 139 insertions(+), 237 deletions(-) create mode 100644 helm/tests/webhook_no_apikey_mount_test.yaml diff --git a/app/config/webhook/settings.go b/app/config/webhook/settings.go index 9510b212e..696bbe15f 100644 --- a/app/config/webhook/settings.go +++ b/app/config/webhook/settings.go @@ -10,10 +10,8 @@ import ( "fmt" "net/url" "os" - "path/filepath" "regexp" "strings" - "sync" "time" "github.com/cloudzero/cloudzero-agent/app/utils/scout" @@ -26,27 +24,30 @@ import ( // Settings represents the configuration settings for the application. type Settings struct { - CloudAccountID string `yaml:"cloud_account_id" env:"CLOUD_ACCOUNT_ID" env-description:"CSP account ID"` - Region string `yaml:"region" env:"CSP_REGION" env-description:"cloud service provider region"` - ClusterName string `yaml:"cluster_name" env:"CLUSTER_NAME" env-description:"name of the cluster to monitor"` - Destination string `yaml:"destination" env:"DESTINATION" env-default:"https://api.cloudzero.com/v1/container-metrics" env-description:"location to send metrics to"` - APIKeyPath string `yaml:"api_key_path" env:"API_KEY_PATH" env-description:"path to the API key file"` - Server Server `yaml:"server"` - Certificate Certificate `yaml:"certificate"` - Logging Logging `yaml:"logging"` - Database Database `yaml:"database"` - Filters Filters `yaml:"filters"` - RemoteWrite RemoteWrite `yaml:"remote_write"` - K8sClient K8sClient `yaml:"k8s_client"` + CloudAccountID string `yaml:"cloud_account_id" env:"CLOUD_ACCOUNT_ID" env-description:"CSP account ID"` + Region string `yaml:"region" env:"CSP_REGION" env-description:"cloud service provider region"` + ClusterName string `yaml:"cluster_name" env:"CLUSTER_NAME" env-description:"name of the cluster to monitor"` + Destination string `yaml:"destination" env:"DESTINATION" env-default:"https://api.cloudzero.com/v1/container-metrics" env-description:"location to send metrics to"` + Server Server `yaml:"server"` + Certificate Certificate `yaml:"certificate"` + Logging Logging `yaml:"logging"` + Database Database `yaml:"database"` + Filters Filters `yaml:"filters"` + RemoteWrite RemoteWrite `yaml:"remote_write"` + K8sClient K8sClient `yaml:"k8s_client"` + + // Deprecated: removed in CP-28161 when the insights-controller stopped + // authenticating to the in-cluster aggregator. Kept as an ignored + // tombstone so legacy configs (older Helm-rendered server-config.yaml, + // or an API_KEY_PATH env var still set in a pod spec) load cleanly under + // strict YAML/env decoders. Has no effect. + APIKeyPath string `yaml:"api_key_path" env:"API_KEY_PATH" env-description:"deprecated; ignored"` + LabelMatches []regexp.Regexp AnnotationMatches []regexp.Regexp - - // control for dynamic reloading - mu sync.Mutex } type RemoteWrite struct { - apiKey string Host string MaxBytesPerSend int `yaml:"max_bytes_per_send" default:"10000000" env:"MAX_BYTES_PER_SEND" env-description:"maximum bytes to send in a single request"` SendInterval time.Duration `yaml:"send_interval" default:"60s" env:"SEND_INTERVAL" env-description:"interval in seconds to send data"` @@ -94,10 +95,6 @@ func NewSettings(configFiles ...string) (*Settings, error) { cfg.setCompiledFilters() - if err := cfg.SetAPIKey(); err != nil { - return nil, fmt.Errorf("failed to get API key: %w", err) - } - cfg.setRemoteWriteURL() cfg.setPolicy() @@ -106,63 +103,11 @@ func NewSettings(configFiles ...string) (*Settings, error) { return &cfg, nil } -func (s *Settings) GetAPIKey() string { - s.mu.Lock() - defer s.mu.Unlock() - return s.RemoteWrite.apiKey -} - -func (s *Settings) SetAPIKey() error { - s.mu.Lock() - defer s.mu.Unlock() - - apiKeyPathLocation, err := absFilePath(s.APIKeyPath) - if err != nil { - return fmt.Errorf("failed to get absolute path: %w", err) - } - - if _, err = os.Stat(apiKeyPathLocation); os.IsNotExist(err) { - return fmt.Errorf("API key file %s not found: %w", apiKeyPathLocation, err) - } - apiKey, err := os.ReadFile(s.APIKeyPath) - if err != nil { - return fmt.Errorf("failed to read API key: %w", err) - } - s.RemoteWrite.apiKey = strings.TrimSpace(string(apiKey)) - - if len(s.RemoteWrite.apiKey) == 0 { - return errors.New("API key is empty") - } - return nil -} - func (s *Settings) setRemoteWriteURL() { - if s.Destination == "" { - s.Destination = "https://api.cloudzero.com/v1/container-metrics" - } - baseURL, err := url.Parse(s.Destination) - if err != nil { - fmt.Println("Malformed URL: ", err.Error()) - return - } - params := url.Values{} - params.Add("cluster_name", s.ClusterName) - params.Add("cloud_account_id", s.CloudAccountID) - params.Add("region", s.Region) - baseURL.RawQuery = params.Encode() - url := baseURL.String() - - if !isValidURL(url) { - log.Fatal().Str("url", url).Msg("URL format invalid") - } - s.RemoteWrite.Host = url -} - -func isValidURL(uri string) bool { - if _, err := url.ParseRequestURI(uri); err != nil { - return false + if _, err := url.ParseRequestURI(s.Destination); err != nil { + log.Fatal().Str("url", s.Destination).Err(err).Msg("URL format invalid") } - return true + s.RemoteWrite.Host = s.Destination } func (s *Settings) setPolicy() { @@ -195,19 +140,6 @@ func (s *Settings) compilePatterns(patterns []string) []regexp.Regexp { return compiledPatterns } -func absFilePath(location string) (string, error) { - dir := filepath.Dir(filepath.Clean(location)) - // validate path if not local directory - if dir == "" || strings.HasPrefix(dir, ".") { - wd, err := os.Getwd() - if err != nil { - return "", fmt.Errorf("failed to get working directory: %w", err) - } - location = filepath.Clean(filepath.Join(wd, location)) - } - return location, nil -} - // Files is a custom flag type to handle multiple configuration files type Files []string diff --git a/app/config/webhook/settings_test.go b/app/config/webhook/settings_test.go index 597e20e45..0dd4cec23 100644 --- a/app/config/webhook/settings_test.go +++ b/app/config/webhook/settings_test.go @@ -5,7 +5,6 @@ package config import ( "os" - "strings" "testing" "time" @@ -17,7 +16,6 @@ func TestNewSettings(t *testing.T) { t.Run("valid config file", func(t *testing.T) { // Create a temporary config file configContent := ` -api_key_path: "/path/to/api_key" server: port: 8080 certificate: @@ -57,21 +55,8 @@ destination: "https://api.cloudzero.com/v1/container-metrics" _, err = tmpFileExtra.Write([]byte(configContentExtra)) require.NoError(t, err) require.NoError(t, tmpFile.Close()) + require.NoError(t, tmpFileExtra.Close()) - // Mock the API key file - apiKeyContent := "test-api-key" - apiKeyFile, err := os.CreateTemp("", "api_key-*.txt") - require.NoError(t, err) - defer os.Remove(apiKeyFile.Name()) - - _, err = apiKeyFile.Write([]byte(apiKeyContent)) - require.NoError(t, err) - require.NoError(t, apiKeyFile.Close()) - - // Update the API key path in the config - configContent = strings.Replace(configContent, "/path/to/api_key", apiKeyFile.Name(), 1) - err = os.WriteFile(tmpFile.Name(), []byte(configContent), 0o644) - require.NoError(t, err) configFiles := Files{tmpFile.Name(), tmpFileExtra.Name()} settings, err := NewSettings(configFiles...) require.NoError(t, err) @@ -85,14 +70,11 @@ destination: "https://api.cloudzero.com/v1/container-metrics" assert.NotEmpty(t, settings.Region, "Region should be set from config or Scout detection") assert.Equal(t, "test-cluster", settings.ClusterName) assert.Equal(t, "https://api.cloudzero.com/v1/container-metrics", settings.Destination) - assert.Equal(t, apiKeyContent, settings.GetAPIKey()) - // Verify RemoteWrite.Host contains the expected base URL and cluster_name - // (cloudAccountId and region may differ due to Scout detection) - assert.Contains(t, settings.RemoteWrite.Host, "https://api.cloudzero.com/v1/container-metrics") - assert.Contains(t, settings.RemoteWrite.Host, "cluster_name=test-cluster") - assert.Contains(t, settings.RemoteWrite.Host, "cloud_account_id="+settings.CloudAccountID) - assert.Contains(t, settings.RemoteWrite.Host, "region="+settings.Region) + // RemoteWrite.Host should equal Destination verbatim — no query-string + // encoding (the aggregator doesn't read those params and we no longer + // authenticate the in-cluster hop). + assert.Equal(t, settings.Destination, settings.RemoteWrite.Host) assert.Equal(t, 10000000, settings.RemoteWrite.MaxBytesPerSend) assert.Equal(t, 60*time.Second, settings.RemoteWrite.SendInterval) diff --git a/app/domain/monitor/secret_monitor_test.go b/app/domain/monitor/secret_monitor_test.go index 3c00c1001..5c8cc89aa 100644 --- a/app/domain/monitor/secret_monitor_test.go +++ b/app/domain/monitor/secret_monitor_test.go @@ -6,13 +6,14 @@ package monitor_test import ( "context" "os" + "strings" + "sync" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - config "github.com/cloudzero/cloudzero-agent/app/config/webhook" "github.com/cloudzero/cloudzero-agent/app/domain/monitor" ) @@ -28,6 +29,31 @@ func (m *MockFileMonitor) Close() { m.Called() } +// fileBackedAPIKey is a minimal monitor.MonitoredAPIKey that reads from a +// file on disk, used here to exercise the secrets monitor's refresh loop. +type fileBackedAPIKey struct { + path string + mu sync.Mutex + key string +} + +func (f *fileBackedAPIKey) GetAPIKey() string { + f.mu.Lock() + defer f.mu.Unlock() + return f.key +} + +func (f *fileBackedAPIKey) SetAPIKey() error { + data, err := os.ReadFile(f.path) + if err != nil { + return err + } + f.mu.Lock() + f.key = strings.TrimSpace(string(data)) + f.mu.Unlock() + return nil +} + func TestSecretsMonitor_Start(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -44,9 +70,7 @@ func TestSecretsMonitor_Start(t *testing.T) { _, err = file.WriteString("foo") assert.NoError(t, err) - settings := &config.Settings{ - APIKeyPath: file.Name(), - } + settings := &fileBackedAPIKey{path: file.Name()} err = settings.SetAPIKey() assert.NoError(t, err) diff --git a/app/domain/pusher/pusher.go b/app/domain/pusher/pusher.go index 7073843d4..b58295f68 100644 --- a/app/domain/pusher/pusher.go +++ b/app/domain/pusher/pusher.go @@ -10,7 +10,6 @@ package pusher import ( "bytes" "context" - "errors" "fmt" "math" "math/rand/v2" @@ -242,18 +241,13 @@ func (h *MetricsPusher) sendBatch(batch []*types.ResourceTags) error { } endpoint := h.settings.RemoteWrite.Host - apiToken := h.settings.GetAPIKey() - if apiToken == "" { - RemoteWriteFailures.WithLabelValues(endpoint).Inc() - return errors.New("API key is empty") - } ts := h.formatMetrics(batch) log.Ctx(h.ctx).Info(). Int("recordCount", len(ts)). Msg("Pushing records to remote write endpoint") - if err := h.pushMetrics(h.settings.RemoteWrite.Host, apiToken, ts); err != nil { + if err := h.pushMetrics(h.settings.RemoteWrite.Host, ts); err != nil { RemoteWriteFailures.WithLabelValues(endpoint).Inc() return fmt.Errorf("failed to push metrics to remote write: %v", err) } @@ -388,7 +382,7 @@ func (h *MetricsPusher) createTimeseries( return ts } -func (h *MetricsPusher) pushMetrics(remoteWriteURL string, apiKey string, timeSeries []prompb.TimeSeries) error { +func (h *MetricsPusher) pushMetrics(remoteWriteURL string, timeSeries []prompb.TimeSeries) error { writeRequest := &prompb.WriteRequest{ Timeseries: timeSeries, } @@ -420,7 +414,6 @@ func (h *MetricsPusher) pushMetrics(remoteWriteURL string, apiKey string, timeSe req.Header.Set("Content-Type", "application/x-protobuf") req.Header.Set("Content-Encoding", "snappy") - req.Header.Set("Authorization", "Bearer "+apiKey) client := &http.Client{} resp, err = client.Do(req) diff --git a/app/domain/pusher/pusher_test.go b/app/domain/pusher/pusher_test.go index 95d9ba1ce..95a6aaae5 100644 --- a/app/domain/pusher/pusher_test.go +++ b/app/domain/pusher/pusher_test.go @@ -9,7 +9,6 @@ import ( "fmt" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -24,30 +23,8 @@ import ( "github.com/cloudzero/cloudzero-agent/app/types/mocks" ) -// createAPIKeyFile creates a temporary API key file with the given content. -// It returns the file path and a cleanup function to remove the file. -func createAPIKeyFile(t *testing.T, apiKeyContent string) string { - apiKeyFile, err := os.CreateTemp("", "api_key-*.txt") - require.NoError(t, err, "Failed to create temp API key file") - - _, err = apiKeyFile.Write([]byte(apiKeyContent)) - require.NoError(t, err, "Failed to write to temp API key file") - - err = apiKeyFile.Close() - require.NoError(t, err, "Failed to close temp API key file") - - t.Cleanup(func() { - _ = os.Remove(apiKeyFile.Name()) - }) - - return apiKeyFile.Name() -} - // setupTest initializes the test environment and returns a TestSetup instance. -func setupTest(t *testing.T, clock types.TimeProvider, store types.ResourceStore, handlerFunc http.HandlerFunc, apiKeyContent string) (*pusher.MetricsPusher, string) { - // Create temporary API key file - apiKeyPath := createAPIKeyFile(t, apiKeyContent) - +func setupTest(t *testing.T, clock types.TimeProvider, store types.ResourceStore, handlerFunc http.HandlerFunc) (*pusher.MetricsPusher, string) { // Start test HTTP server server := httptest.NewServer(http.HandlerFunc(handlerFunc)) t.Cleanup(func() { @@ -56,7 +33,6 @@ func setupTest(t *testing.T, clock types.TimeProvider, store types.ResourceStore // Initialize settings settings := &config.Settings{ - APIKeyPath: apiKeyPath, RemoteWrite: config.RemoteWrite{ Host: server.URL, MaxBytesPerSend: 40, // small - should be no more than 2 objects at a time (see mkRecords) @@ -65,7 +41,6 @@ func setupTest(t *testing.T, clock types.TimeProvider, store types.ResourceStore MaxRetries: 3, }, } - settings.SetAPIKey() p := pusher.New(context.Background(), store, clock, settings) rw := p.(*pusher.MetricsPusher) @@ -100,7 +75,7 @@ func Test_SupportsRunnableInterface(t *testing.T) { defer ctrl.Finish() mockStore := mocks.NewMockResourceStore(ctrl) - p, _ := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}, "") + p, _ := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}) assert.False(t, p.IsRunning()) mockStore.EXPECT().FindAllBy(gomock.Any(), gomock.Any()).Return([]*types.ResourceTags{}, nil).AnyTimes() @@ -137,7 +112,6 @@ func Test_FlushMany(t *testing.T) { })).Return(nil).AnyTimes() // Capture the records that were sent - apiKeyContent := "apiKeyContent" expectedSentCount := 3 actualSentCount := 0 p, host := setupTest(t, mockClock, mockStore, @@ -147,8 +121,10 @@ func Test_FlushMany(t *testing.T) { if r.Method != http.MethodPost { t.Errorf("Expected POST method, got: %s", r.Method) } - if r.Header.Get("Authorization") != fmt.Sprintf("Bearer %s", apiKeyContent) { - t.Errorf("Expected Authorization header to be 'Bearer %s', got: %s", apiKeyContent, r.Header.Get("Authorization")) + // The aggregator does not authenticate the in-cluster hop; + // the webhook must not send an Authorization header. + if got := r.Header.Get("Authorization"); got != "" { + t.Errorf("Expected no Authorization header, got: %q", got) } if r.Header.Get("Content-Encoding") != "snappy" { t.Errorf("Expected Content-Encoding 'snappy', got: %s", r.Header.Get("Content-Encoding")) @@ -158,7 +134,6 @@ func Test_FlushMany(t *testing.T) { } w.WriteHeader(http.StatusOK) }, - apiKeyContent, ) err := p.Flush() @@ -190,7 +165,7 @@ func Test_Flush_FindAll_ReturnsNothing(t *testing.T) { emptyList := mkRecords(currentTime, 0) mockStore.EXPECT().FindAllBy(gomock.Any(), gomock.Any()).Return(emptyList, nil).Times(1) - p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}, "apiKeyContent") + p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}) err := p.Flush() assert.NoError(t, err) @@ -219,7 +194,7 @@ func Test_Flush_Handles_FindAll_Error_Gracefully(t *testing.T) { expectedError := errors.New("find error") mockStore.EXPECT().FindAllBy(gomock.Any(), gomock.Any()).Return(nil, expectedError).Times(1) - p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}, "apiKeyContent") + p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}) err := p.Flush() assert.Error(t, err) @@ -252,7 +227,7 @@ func Test_Flush_Handles_Tx_Error_Gracefully(t *testing.T) { expectedError := errors.New("find error") mockStore.EXPECT().Tx(gomock.Any(), gomock.Any()).Return(expectedError).Times(1) - p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}, "apiKeyContent") + p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}) err := p.Flush() assert.Error(t, err) @@ -286,7 +261,7 @@ func Test_Flush_Handles_Update_Error_Gracefully(t *testing.T) { expectedError := errors.New("find error") mockStore.EXPECT().Update(gomock.Any(), gomock.Any()).Return(expectedError).Times(1) - p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}, "apiKeyContent") + p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) {}) err := p.Flush() assert.Error(t, err) @@ -315,13 +290,10 @@ func Test_Flush_Handles_SendFailure(t *testing.T) { records := mkRecords(currentTime, 1) mockStore.EXPECT().FindAllBy(gomock.Any(), gomock.Any()).Return(records, nil).AnyTimes() - // Capture the records that were sent - apiKeyContent := "apiKeyContent" p, host := setupTest(t, mockClock, mockStore, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) }, - apiKeyContent, ) err := p.Flush() diff --git a/app/functions/webhook/main.go b/app/functions/webhook/main.go index cafb0fd5c..4d81b3e8f 100644 --- a/app/functions/webhook/main.go +++ b/app/functions/webhook/main.go @@ -58,7 +58,7 @@ func main() { Str("chartsRepo", build.ChartsRepo). Str("platformEndpoint", settings.Destination). Interface("configFiles", configFiles). - Msg("Starting CloudZero Insights Controller") + Msg("Starting CloudZero Webhook Server") if len(configFiles) == 0 { log.Fatal().Msg("No configuration files provided") } @@ -87,24 +87,13 @@ func main() { log.Fatal().Err(err).Msg("Failed to create in-memory resource repository") } - // Start a monitor that can pickup secrets changes and update the settings ctx, cancel := context.WithCancel(context.Background()) defer cancel() - secretMon := monitor.NewSecretMonitor(ctx, settings) - if err = secretMon.Run(); err != nil { - log.Fatal().Err(err).Msg("failed to run secret monitor") //nolint:gocritic // It's okay if the `defer cancel()` doesn't run since we're exiting. - } - defer func() { - if innerErr := secretMon.Shutdown(); innerErr != nil { - log.Err(innerErr).Msg("failed to shut down secret monitor") - } - }() - // create remote metrics writer dataPusher := pusher.New(ctx, store, clock, settings) if err = dataPusher.Run(); err != nil { - log.Fatal().Err(err).Msg("failed to start remote metrics writer") + log.Fatal().Err(err).Msg("failed to start remote metrics writer") //nolint:gocritic // It's okay if the `defer cancel()` doesn't run since we're exiting. } defer func() { log.Ctx(ctx).Debug().Msg("Starting main shutdown process") diff --git a/helm/templates/_cm_helpers.tpl b/helm/templates/_cm_helpers.tpl index de313cbbc..a82e35293 100644 --- a/helm/templates/_cm_helpers.tpl +++ b/helm/templates/_cm_helpers.tpl @@ -136,7 +136,6 @@ database: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 -api_key_path: {{ include "cloudzero-agent.secretFileFullPath" . }} {{- $namespace := .Release.Namespace }} {{- with .Values.insightsController }} certificate: diff --git a/helm/templates/webhook-deploy.yaml b/helm/templates/webhook-deploy.yaml index 0afd024be..a5c33ff85 100644 --- a/helm/templates/webhook-deploy.yaml +++ b/helm/templates/webhook-deploy.yaml @@ -137,12 +137,6 @@ spec: mountPath: {{ .Values.insightsController.tls.mountPath }} readOnly: true {{- end }} - {{- if or .Values.existingSecretName .Values.apiKey }} - - name: cloudzero-api-key - mountPath: {{ .Values.serverConfig.containerSecretFilePath }} - subPath: "" - readOnly: true - {{- end }} {{- with .Values.insightsController.volumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} @@ -179,11 +173,6 @@ spec: secret: secretName: {{ include "cloudzero-agent.tlsSecretName" . }} {{- end }} - {{- if or .Values.existingSecretName .Values.apiKey }} - - name: cloudzero-api-key - secret: - secretName: {{ include "cloudzero-agent.secretName" . }} - {{- end }} {{- with .Values.insightsController.volumes }} {{- toYaml . | nindent 8 }} {{- end }} diff --git a/helm/tests/webhook_no_apikey_mount_test.yaml b/helm/tests/webhook_no_apikey_mount_test.yaml new file mode 100644 index 000000000..4dfaec4b4 --- /dev/null +++ b/helm/tests/webhook_no_apikey_mount_test.yaml @@ -0,0 +1,70 @@ +suite: webhook (insights-controller) no longer mounts the API key +templates: + - templates/webhook-deploy.yaml + - templates/webhook-cm.yaml +tests: + - it: webhook Deployment has no cloudzero-api-key volume + template: templates/webhook-deploy.yaml + set: + apiKey: "test-key-abc123" + existingSecretName: null + asserts: + - notContains: + path: spec.template.spec.volumes + content: + name: cloudzero-api-key + any: true + + - it: webhook container has no cloudzero-api-key volumeMount + template: templates/webhook-deploy.yaml + set: + apiKey: "test-key-abc123" + existingSecretName: null + asserts: + - notContains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: cloudzero-api-key + any: true + + - it: webhook Deployment also has no api-key mount when existingSecretName is used + template: templates/webhook-deploy.yaml + set: + apiKey: null + existingSecretName: "my-existing-secret" + asserts: + - notContains: + path: spec.template.spec.volumes + content: + name: cloudzero-api-key + any: true + - notContains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: cloudzero-api-key + any: true + + - it: webhook server-config.yaml does not reference api_key_path + template: templates/webhook-cm.yaml + set: + apiKey: "test-key-abc123" + existingSecretName: null + asserts: + - notMatchRegex: + path: data["server-config.yaml"] + pattern: "api_key_path" + + - it: webhook server-config.yaml does not encode cluster info as query string + # The aggregator does not parse those query params and the webhook now + # POSTs straight to the in-cluster destination URL. + template: templates/webhook-cm.yaml + set: + apiKey: "test-key-abc123" + existingSecretName: null + asserts: + - notMatchRegex: + path: data["server-config.yaml"] + pattern: "cluster_name=" + - notMatchRegex: + path: data["server-config.yaml"] + pattern: "cloud_account_id=" diff --git a/tests/helm/template/alloy.yaml b/tests/helm/template/alloy.yaml index 8ed6edc7f..16699401c 100644 --- a/tests/helm/template/alloy.yaml +++ b/tests/helm/template/alloy.yaml @@ -1894,7 +1894,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -3043,10 +3042,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -3074,9 +3069,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1 diff --git a/tests/helm/template/cert-manager.yaml b/tests/helm/template/cert-manager.yaml index ded942b32..a4d88bff8 100644 --- a/tests/helm/template/cert-manager.yaml +++ b/tests/helm/template/cert-manager.yaml @@ -1809,7 +1809,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -2937,10 +2936,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -2968,9 +2963,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1 diff --git a/tests/helm/template/federated.yaml b/tests/helm/template/federated.yaml index 3bf4a2789..8f4a67b64 100644 --- a/tests/helm/template/federated.yaml +++ b/tests/helm/template/federated.yaml @@ -1897,7 +1897,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -3220,10 +3219,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -3251,9 +3246,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1 diff --git a/tests/helm/template/istio.yaml b/tests/helm/template/istio.yaml index df4965bfc..403c14920 100644 --- a/tests/helm/template/istio.yaml +++ b/tests/helm/template/istio.yaml @@ -1824,7 +1824,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -2953,10 +2952,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -2984,9 +2979,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1 diff --git a/tests/helm/template/kubestate.yaml b/tests/helm/template/kubestate.yaml index 57b6e7ba6..61a6f5e1b 100644 --- a/tests/helm/template/kubestate.yaml +++ b/tests/helm/template/kubestate.yaml @@ -1638,7 +1638,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -2493,10 +2492,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -2524,9 +2519,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1 diff --git a/tests/helm/template/manifest.yaml b/tests/helm/template/manifest.yaml index 16ef82b47..c14c33f04 100644 --- a/tests/helm/template/manifest.yaml +++ b/tests/helm/template/manifest.yaml @@ -1824,7 +1824,6 @@ data: retention_time: 24h cleanup_interval: 3h batch_update_size: 500 - api_key_path: /etc/config/secrets/value certificate: key: /etc/certs/tls.key cert: /etc/certs/tls.crt @@ -2952,10 +2951,6 @@ spec: - name: tls-certs mountPath: /etc/certs readOnly: true - - name: cloudzero-api-key - mountPath: /etc/config/secrets/ - subPath: "" - readOnly: true livenessProbe: httpGet: scheme: HTTPS @@ -2983,9 +2978,6 @@ spec: - name: tls-certs secret: secretName: cz-agent-cz-webhook-tls - - name: cloudzero-api-key - secret: - secretName: cz-agent-api-key --- # Source: cloudzero-agent/templates/backfill-job.yaml apiVersion: batch/v1