Skip to content

Commit

Permalink
feat: allow sanitizing configuration sent to Konnect (#5489)
Browse files Browse the repository at this point in the history
Co-authored-by: Jakub Warczarek <jakub.warczarek@konghq.com>
  • Loading branch information
czeslavo and programmer04 authored Jan 26, 2024
1 parent 44227f1 commit 4df8f2a
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 24 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 8 additions & 7 deletions FEATURE_GATES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
45 changes: 44 additions & 1 deletion internal/dataplane/kong_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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")
}
5 changes: 4 additions & 1 deletion internal/dataplane/kongstate/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion internal/dataplane/kongstate/license.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions internal/dataplane/sendconfig/kong.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion internal/dataplane/sendconfig/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func (r DefaultUpdateStrategyResolver) resolveUpdateStrategy(client UpdateClient
return NewUpdateStrategyDBModeKonnect(
adminAPIClient,
dump.Config{
SkipCACerts: true,
KonnectControlPlane: client.KonnectControlPlane(),
},
r.config.Version,
Expand Down
12 changes: 8 additions & 4 deletions internal/manager/featuregates/feature_gates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
}
}
15 changes: 8 additions & 7 deletions internal/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 7 additions & 1 deletion test/envtest/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;"+
Expand All @@ -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:
Expand Down

0 comments on commit 4df8f2a

Please sign in to comment.