diff --git a/CHANGELOG.md b/CHANGELOG.md index b052fc282d..333d91a4c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -157,9 +157,12 @@ Adding a new version? You'll need three changes: It's only possible to set the same timeout for each rule in a single `HTTPRoute`. Other settings will be rejected by the admission webhook validation. [#5243](https://github.com/Kong/kubernetes-ingress-controller/pull/5243) -- Log the details in response fron Konnect when failed to push configuration +- Log the details in response from Konnect when failed to push configuration to Konnect. [#5453](https://github.com/Kong/kubernetes-ingress-controller/pull/5453) +- Added `SanitizeKonnectConfigDumps` feature gate allowing to enable sanitizing + sensitive data in Konnect configuration dumps. + [#5489](https://github.com/Kong/kubernetes-ingress-controller/pull/5489) ### Fixed diff --git a/FEATURE_GATES.md b/FEATURE_GATES.md index 3ed18808ca..b003bbb3a7 100644 --- a/FEATURE_GATES.md +++ b/FEATURE_GATES.md @@ -62,13 +62,14 @@ Features that reach GA and over time become stable will be removed from this tab ### Feature gates for Alpha or Beta features -| Feature | Default | Stage | Since | Until | -|-------------------|---------|-------|--------|-------| -| GatewayAlpha | `false` | Alpha | 2.6.0 | TBD | -| FillIDs | `false` | Alpha | 2.10.0 | 3.0.0 | -| FillIDs | `true` | Beta | 3.0.0 | TBD | -| RewriteURIs | `false` | Alpha | 2.12.0 | TBD | -| KongServiceFacade | `false` | Alpha | 3.1.0 | TBD | +| Feature | Default | Stage | Since | Until | +|----------------------------|---------|-------|--------|-------| +| GatewayAlpha | `false` | Alpha | 2.6.0 | TBD | +| FillIDs | `false` | Alpha | 2.10.0 | 3.0.0 | +| FillIDs | `true` | Beta | 3.0.0 | TBD | +| RewriteURIs | `false` | Alpha | 2.12.0 | TBD | +| KongServiceFacade | `false` | Alpha | 3.1.0 | TBD | +| SanitizeKonnectConfigDumps | `false` | Alpha | 3.1.0 | TBD | **NOTE**: The `Gateway` feature gate refers to [Gateway API](https://github.com/kubernetes-sigs/gateway-api) APIs which are in diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go index 4fe828f1fc..b6fc3a8d60 100644 --- a/internal/dataplane/kong_client.go +++ b/internal/dataplane/kong_client.go @@ -550,6 +550,11 @@ func (c *KongClient) sendToClient( ) (string, error) { logger := c.logger.WithValues("url", client.AdminAPIClient().BaseRootURL()) + // If the client is Konnect and the feature flag is turned on, + // we should sanitize the configuration before sending it out. + if client.IsKonnect() && config.SanitizeKonnectConfigDumps { + s = s.SanitizedCopy() + } deckGenParams := deckgen.GenerateDeckContentParams{ SelectorTags: config.FilterTags, ExpressionRoutes: config.ExpressionRoutes, diff --git a/internal/dataplane/kong_client_test.go b/internal/dataplane/kong_client_test.go index 36cd0a3a41..7ddc2c7734 100644 --- a/internal/dataplane/kong_client_test.go +++ b/internal/dataplane/kong_client_test.go @@ -680,7 +680,9 @@ func setupTestKongClient( logger := zapr.NewLogger(zap.NewNop()) timeout := time.Second diagnostic := util.ConfigDumpDiagnostic{} - config := sendconfig.Config{} + config := sendconfig.Config{ + SanitizeKonnectConfigDumps: true, + } if eventRecorder == nil { eventRecorder = mocks.NewEventRecorder() @@ -884,3 +886,44 @@ func TestKongClientUpdate_FetchStoreAndPushLastValidConfig(t *testing.T) { }) } } + +func TestKongClientUpdate_KonnectUpdatesAreSanitized(t *testing.T) { + ctx := context.Background() + clientsProvider := mockGatewayClientsProvider{ + gatewayClients: []*adminapi.Client{mustSampleGatewayClient(t)}, + konnectClient: mustSampleKonnectClient(t), + } + updateStrategyResolver := newMockUpdateStrategyResolver(t) + configChangeDetector := mockConfigurationChangeDetector{hasConfigurationChanged: true} + configBuilder := newMockKongConfigBuilder() + configBuilder.kongState = &kongstate.KongState{ + Certificates: []kongstate.Certificate{ + { + Certificate: kong.Certificate{ + ID: kong.String("new_cert"), + Key: kong.String(`private-key-string`), // This should be redacted. + }, + }, + }, + } + + kongRawStateGetter := &mockKongLastValidConfigFetcher{} + kongClient := setupTestKongClient( + t, + updateStrategyResolver, + clientsProvider, + configChangeDetector, + configBuilder, + nil, + kongRawStateGetter, + ) + + require.NoError(t, kongClient.Update(ctx)) + + konnectContent, ok := updateStrategyResolver.lastUpdatedContentForURL(clientsProvider.konnectClient.BaseRootURL()) + require.True(t, ok, "expected Konnect to be updated") + require.Len(t, konnectContent.Content.Certificates, 1, "expected Konnect to have 1 certificate") + cert := konnectContent.Content.Certificates[0] + require.NotNil(t, cert.Key, "expected Konnect to have certificate key") + require.Equal(t, "{vault://redacted-value}", *cert.Key, "expected Konnect to have redacted certificate key") +} diff --git a/internal/dataplane/kongstate/credentials.go b/internal/dataplane/kongstate/credentials.go index 07c0f681bf..48d9b4d608 100644 --- a/internal/dataplane/kongstate/credentials.go +++ b/internal/dataplane/kongstate/credentials.go @@ -7,7 +7,10 @@ import ( "github.com/mitchellh/mapstructure" ) -var redactedString = kong.String("REDACTED") +// redactedString is used to redact sensitive values in the KongState. +// It uses a vault URI to pass Konnect Admin API validations (e.g. when a TLS key is expected, it's only possible +// to pass a valid key or a vault URI). +var redactedString = kong.String("{vault://redacted-value}") // KeyAuth represents a key-auth credential. type KeyAuth struct { diff --git a/internal/dataplane/kongstate/license.go b/internal/dataplane/kongstate/license.go index 36faa14da9..2d714cbb6b 100644 --- a/internal/dataplane/kongstate/license.go +++ b/internal/dataplane/kongstate/license.go @@ -1,6 +1,8 @@ package kongstate -import "github.com/kong/go-kong/kong" +import ( + "github.com/kong/go-kong/kong" +) // License represents the license object in Kong. type License struct { diff --git a/internal/dataplane/sendconfig/kong.go b/internal/dataplane/sendconfig/kong.go index e8de3b4c6a..32163a8d50 100644 --- a/internal/dataplane/sendconfig/kong.go +++ b/internal/dataplane/sendconfig/kong.go @@ -37,6 +37,9 @@ type Config struct { // ExpressionRoutes indicates whether to use Kong's expression routes. ExpressionRoutes bool + + // SanitizeKonnectConfigDumps indicates whether to sanitize Konnect config dumps. + SanitizeKonnectConfigDumps bool } // Init sets up variables that need external calls. diff --git a/internal/dataplane/sendconfig/strategy.go b/internal/dataplane/sendconfig/strategy.go index 7feafcc195..cb3744c5ba 100644 --- a/internal/dataplane/sendconfig/strategy.go +++ b/internal/dataplane/sendconfig/strategy.go @@ -93,7 +93,6 @@ func (r DefaultUpdateStrategyResolver) resolveUpdateStrategy(client UpdateClient return NewUpdateStrategyDBModeKonnect( adminAPIClient, dump.Config{ - SkipCACerts: true, KonnectControlPlane: client.KonnectControlPlane(), }, r.config.Version, diff --git a/internal/manager/featuregates/feature_gates.go b/internal/manager/featuregates/feature_gates.go index a003dcbc12..569350674c 100644 --- a/internal/manager/featuregates/feature_gates.go +++ b/internal/manager/featuregates/feature_gates.go @@ -25,6 +25,9 @@ const ( // KongServiceFacade is the name of the feature-gate for enabling KongServiceFacade CR reconciliation. KongServiceFacade = "KongServiceFacade" + // SanitizeKonnectConfigDumps is the name of the feature-gate that enables sanitization of Konnect config dumps. + SanitizeKonnectConfigDumps = "SanitizeKonnectConfigDumps" + // DocsURL provides a link to the documentation for feature gates in the KIC repository. DocsURL = "https://github.com/Kong/kubernetes-ingress-controller/blob/main/FEATURE_GATES.md" ) @@ -60,9 +63,10 @@ func (fg FeatureGates) Enabled(feature string) bool { // NOTE: if you're adding a new feature gate, it needs to be added here. func GetFeatureGatesDefaults() FeatureGates { return map[string]bool{ - GatewayAlphaFeature: false, - FillIDsFeature: true, - RewriteURIsFeature: false, - KongServiceFacade: false, + GatewayAlphaFeature: false, + FillIDsFeature: true, + RewriteURIsFeature: false, + KongServiceFacade: false, + SanitizeKonnectConfigDumps: false, } } diff --git a/internal/manager/run.go b/internal/manager/run.go index 0a4ac2863b..3d90ffaa5e 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -116,13 +116,14 @@ func Run( kongSemVersion := semver.Version{Major: v.Major(), Minor: v.Minor(), Patch: v.Patch()} kongConfig := sendconfig.Config{ - Version: kongSemVersion, - InMemory: dbMode.IsDBLessMode(), - Concurrency: c.Concurrency, - FilterTags: c.FilterTags, - SkipCACertificates: c.SkipCACertificates, - EnableReverseSync: c.EnableReverseSync, - ExpressionRoutes: dpconf.ShouldEnableExpressionRoutes(routerFlavor), + Version: kongSemVersion, + InMemory: dbMode.IsDBLessMode(), + Concurrency: c.Concurrency, + FilterTags: c.FilterTags, + SkipCACertificates: c.SkipCACertificates, + EnableReverseSync: c.EnableReverseSync, + ExpressionRoutes: dpconf.ShouldEnableExpressionRoutes(routerFlavor), + SanitizeKonnectConfigDumps: featureGates.Enabled(featuregates.SanitizeKonnectConfigDumps), } kongConfig.Init(ctx, setupLog, initialKongClients) diff --git a/test/envtest/telemetry_test.go b/test/envtest/telemetry_test.go index 61c2ea74e7..a0319223ba 100644 --- a/test/envtest/telemetry_test.go +++ b/test/envtest/telemetry_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -361,6 +362,7 @@ func verifyTelemetryReport(t *testing.T, k8sVersion *version.Info, report string "feature-kongservicefacade=false;"+ "feature-konnect-sync=false;"+ "feature-rewriteuris=false;"+ + "feature-sanitizekonnectconfigdumps=false;"+ "hn=%s;"+ "kv=3.4.1;"+ "rf=traditional;"+ @@ -386,7 +388,11 @@ func verifyTelemetryReport(t *testing.T, k8sVersion *version.Info, report string k8sVersion.GitVersion, "v"+semver.String(), ) - return report == expectedReport + if diff := cmp.Diff(expectedReport, report); diff != "" { + t.Logf("telemetry report mismatch (-want +got):\n%s", diff) + return false + } + return true } // removeStanzaFromReport removes stanza from report. Report contains stanzas like: