Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow sanitizing configuration sent to Konnect #5489

Merged
merged 2 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading