Skip to content

Commit

Permalink
use webhook options
Browse files Browse the repository at this point in the history
  • Loading branch information
skonto committed Jul 7, 2023
1 parent 9f240b9 commit 9108646
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 24 deletions.
6 changes: 3 additions & 3 deletions webhook/admission_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestAdmissionValidResponseForResourceTLS(t *testing.T) {
path: "/bazinga",
response: &admissionv1.AdmissionResponse{},
}
wh, serverURL, ctx, cancel, err := testSetup(t, ac)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, ac)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down Expand Up @@ -353,7 +353,7 @@ func TestAdmissionInvalidResponseForResource(t *testing.T) {
path: "/booger",
response: MakeErrorStatus(expectedError),
}
wh, serverURL, ctx, cancel, err := testSetup(t, ac)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, ac)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down Expand Up @@ -470,7 +470,7 @@ func TestAdmissionWarningResponseForResource(t *testing.T) {
path: "/warnmeplease",
response: &admissionv1.AdmissionResponse{Warnings: []string{"everything is not fine.\nlike really\nfor sure"}},
}
wh, serverURL, ctx, cancel, err := testSetup(t, ac)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, ac)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down
12 changes: 6 additions & 6 deletions webhook/certificates/resources/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package resources

import (
"context"
"os"
"time"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -66,14 +65,15 @@ func MakeSecretInternal(ctx context.Context, name, namespace, serviceName string
}

// GetSecretDataKeyNamesOrDefault gets the names of the keys in the webhook secret's data.
func GetSecretDataKeyNamesOrDefault() (serverKey string, serverCert string) {
func GetSecretDataKeyNamesOrDefault(sKey string, sCert string) (serverKey string, serverCert string) {
serverKey = ServerKey
serverCert = ServerCert
if val, ok := os.LookupEnv(ServerKeyEnv); ok {
serverKey = val

if sKey != "" {
serverKey = sKey
}
if val, ok := os.LookupEnv(ServerCertEnv); ok {
serverCert = val
if sCert != "" {
serverCert = sCert
}
return serverKey, serverCert
}
4 changes: 2 additions & 2 deletions webhook/conversion_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestConversionValidResponse(t *testing.T) {
UID: types.UID("some-uid"),
},
}
wh, serverURL, ctx, cancel, err := testSetup(t, cc)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, cc)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down Expand Up @@ -153,7 +153,7 @@ func TestConversionInvalidResponse(t *testing.T) {
},
},
}
wh, serverURL, ctx, cancel, err := testSetup(t, cc)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, cc)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down
5 changes: 3 additions & 2 deletions webhook/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func createSecureTLSClient(t *testing.T, kubeClient kubernetes.Interface, acOpts
return nil, err
}

sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault()
sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault(acOpts.ServerKey, acOpts.ServerCert)

serverKey := secret.Data[sKey]
serverCert := secret.Data[sCert]
Expand Down Expand Up @@ -248,7 +248,8 @@ func customSecretWithOverrides(ctx context.Context, name, namespace, serviceName
if err != nil {
return nil, err
}
sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault()
webOpts := GetOptions(ctx)
sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault(webOpts.ServerKey, webOpts.ServerCert)
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand Down
11 changes: 10 additions & 1 deletion webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ type Options struct {
// If no SecretName is provided, then the webhook serves without TLS.
SecretName string

// ServerKeyEnv is the env var name for the webhook secret's key eg. `tls.key`.
// Default value is `server-key.pem`.
ServerKey string

// ServerCertEnv is the env var name for the webhook secret's ca data key eg. `tls.crt`.
// Default value is `server-cert.pem`.
ServerCert 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,7 +177,8 @@ func New(
logger.Errorw("failed to fetch secret", zap.Error(err))
return nil, nil
}
sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault()
webOpts := GetOptions(ctx)
sKey, sCert := certresources.GetSecretDataKeyNamesOrDefault(webOpts.ServerKey, webOpts.ServerCert)
serverKey, ok := secret.Data[sKey]
if !ok {
logger.Warn("server key missing")
Expand Down
23 changes: 13 additions & 10 deletions webhook/webhook_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func createResource(name string) *pkgtest.Resource {
const testTimeout = 10 * time.Second

func TestMissingContentType(t *testing.T) {
wh, serverURL, ctx, cancel, err := testSetup(t)
wh, serverURL, ctx, cancel, err := testSetup(t, nil)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down Expand Up @@ -106,7 +106,10 @@ func TestMissingContentType(t *testing.T) {
}

func TestMissingContentTypeCustomSecret(t *testing.T) {
wh, serverURL, ctx, cancel, err := testSetup(t)
defaultOptions := newCustomOptions()
certresources.MakeSecret = customSecretWithOverrides

wh, serverURL, ctx, cancel, err := testSetup(t, &defaultOptions)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand All @@ -126,10 +129,6 @@ func TestMissingContentTypeCustomSecret(t *testing.T) {
t.Fatal("waitForServerAvailable() =", err)
}

t.Setenv(certresources.ServerKeyEnv, "tls.key")
t.Setenv(certresources.ServerCertEnv, "tls.crt")
certresources.MakeSecret = customSecretWithOverrides

defer func() {
certresources.MakeSecret = certresources.MakeSecretInternal
}()
Expand Down Expand Up @@ -168,7 +167,7 @@ func TestMissingContentTypeCustomSecret(t *testing.T) {
}

func testEmptyRequestBody(t *testing.T, controller interface{}) {
wh, serverURL, ctx, cancel, err := testSetup(t, controller)
wh, serverURL, ctx, cancel, err := testSetup(t, nil, controller)
if err != nil {
t.Fatal("testSetup() =", err)
}
Expand Down Expand Up @@ -253,7 +252,7 @@ func TestSetupWebhookHTTPServerError(t *testing.T) {
}
}

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

// ephemeral port
Expand All @@ -262,8 +261,12 @@ func testSetup(t *testing.T, acs ...interface{}) (*Webhook, string, context.Cont
t.Fatal("unable to get ephemeral port: ", err)
}

defaultOpts := newDefaultOptions()

var defaultOpts Options
if options == nil {
defaultOpts = newDefaultOptions()
} else {
defaultOpts = *options
}
ctx, wh, cancel := newNonRunningTestWebhook(t, defaultOpts, acs...)
wh.testListener = l

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",
ServerKey: "tls.key",
ServerCert: "tls.crt",
}
}

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

0 comments on commit 9108646

Please sign in to comment.