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

Allow overriding webhook secret data keys #2662

Merged
merged 8 commits into from
Aug 2, 2023
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
14 changes: 14 additions & 0 deletions webhook/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/webhook/certificates/resources"
)

// EnsureLabelSelectorExpressions merges the current label selector's MatchExpressions
Expand Down Expand Up @@ -66,3 +67,16 @@ func ensureLabelSelectorRequirements(

return append(want, nonKnative...)
}

func getSecretDataKeyNamesOrDefault(sKey string, sCert string) (serverKey string, serverCert string) {
serverKey = resources.ServerKey
serverCert = resources.ServerCert

if sKey != "" {
serverKey = sKey
}
if sCert != "" {
serverCert = sCert
}
return serverKey, serverCert
}
26 changes: 24 additions & 2 deletions webhook/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,10 @@ func createSecureTLSClient(t *testing.T, kubeClient kubernetes.Interface, acOpts
return nil, err
}

serverKey := secret.Data[certresources.ServerKey]
serverCert := secret.Data[certresources.ServerCert]
sKey, sCert := getSecretDataKeyNamesOrDefault(acOpts.ServerPrivateKeyName, acOpts.ServerCertificateName)

serverKey := secret.Data[sKey]
serverCert := secret.Data[sCert]
caCert := secret.Data[certresources.CACert]

// Build cert pool with CA Cert
Expand Down Expand Up @@ -240,3 +242,23 @@ func createNonTLSClient() *http.Client {
Transport: &http.Transport{},
}
}

func customSecretWithOverrides(ctx context.Context, name, namespace, serviceName string) (*corev1.Secret, error) {
serverKey, serverCert, caCert, err := certresources.CreateCerts(ctx, serviceName, namespace, time.Now().Add(24*time.Hour))
if err != nil {
return nil, err
}
webOpts := GetOptions(ctx)
sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName)
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Data: map[string][]byte{
sKey: serverKey,
sCert: serverCert,
certresources.CACert: caCert,
},
}, nil
}
16 changes: 12 additions & 4 deletions webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
admissionv1 "k8s.io/api/admission/v1"
"knative.dev/pkg/logging"
"knative.dev/pkg/system"
certresources "knative.dev/pkg/webhook/certificates/resources"
)

// Options contains the configuration for the webhook
Expand All @@ -58,6 +57,14 @@ type Options struct {
// If no SecretName is provided, then the webhook serves without TLS.
SecretName string

// ServerPrivateKeyName is the name for the webhook secret's data key e.g. `tls.key`.
// Default value is `server-key.pem` if no value is passed.
ServerPrivateKeyName string

// ServerCertificateName is the name for the webhook secret's ca data key e.g. `tls.crt`.
// Default value is `server-cert.pem` if no value is passed.
ServerCertificateName string

// Port where the webhook is served. Per k8s admission
// registration requirements this should be 443 unless there is
// only a single port for the service.
Expand Down Expand Up @@ -169,13 +176,14 @@ func New(
logger.Errorw("failed to fetch secret", zap.Error(err))
return nil, nil
}

serverKey, ok := secret.Data[certresources.ServerKey]
webOpts := GetOptions(ctx)
sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName)
serverKey, ok := secret.Data[sKey]
if !ok {
logger.Warn("server key missing")
return nil, nil
}
serverCert, ok := secret.Data[certresources.ServerCert]
serverCert, ok := secret.Data[sCert]
if !ok {
logger.Warn("server cert missing")
return nil, nil
Expand Down
51 changes: 51 additions & 0 deletions webhook/webhook_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@ func TestMissingContentType(t *testing.T) {
metricstest.CheckStatsNotReported(t, requestCountName, requestLatenciesName)
}

func TestServerWithCustomSecret(t *testing.T) {
wh, serverURL, ctx, cancel, err := testSetupCustomSecret(t)
if err != nil {
t.Fatal("testSetup() =", err)
}

eg, _ := errgroup.WithContext(ctx)
eg.Go(func() error { return wh.Run(ctx.Done()) })
wh.InformersHaveSynced()
defer func() {
cancel()
if err := eg.Wait(); err != nil {
t.Error("Unable to run controller:", err)
}
}()

pollErr := waitForServerAvailable(t, serverURL, testTimeout)
if pollErr != nil {
t.Fatal("waitForServerAvailable() =", err)
}
}

func testEmptyRequestBody(t *testing.T, controller interface{}) {
wh, serverURL, ctx, cancel, err := testSetup(t, controller)
if err != nil {
Expand Down Expand Up @@ -220,6 +242,35 @@ func testSetup(t *testing.T, acs ...interface{}) (*Webhook, string, context.Cont
return wh, l.Addr().String(), ctx, cancel, nil
}

func testSetupCustomSecret(t *testing.T, acs ...interface{}) (*Webhook, string, context.Context, context.CancelFunc, error) {
t.Helper()

// ephemeral port
l, err := net.Listen("tcp", ":0")
if err != nil {
t.Fatal("unable to get ephemeral port: ", err)
}

defaultOptions := newCustomOptions()

ctx, wh, cancel := newNonRunningTestWebhook(t, defaultOptions, acs...)
wh.testListener = l

// Create certificate
secret, err := customSecretWithOverrides(ctx, defaultOptions.SecretName, system.Namespace(), defaultOptions.ServiceName)
if err != nil {
t.Fatalf("failed to create certificate")
}
kubeClient := kubeclient.Get(ctx)

if _, err := kubeClient.CoreV1().Secrets(secret.Namespace).Create(context.Background(), secret, metav1.CreateOptions{}); err != nil {
t.Fatalf("failed to create secret")
}

resetMetrics()
return wh, l.Addr().String(), ctx, cancel, nil
}

func testSetupNoTLS(t *testing.T, acs ...interface{}) (*Webhook, string, context.Context, context.CancelFunc, error) {
t.Helper()

Expand Down
10 changes: 10 additions & 0 deletions webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ func newDefaultOptions() Options {
}
}

func newCustomOptions() Options {
return Options{
ServiceName: "webhook",
Port: 8443,
SecretName: "webhook-certs",
ServerPrivateKeyName: "tls.key",
ServerCertificateName: "tls.crt",
}
}

const (
testResourceName = "test-resource"
user1 = "brutto@knative.dev"
Expand Down