diff --git a/config/crd/bases/runtime.cluster.x-k8s.io_extensionconfigs.yaml b/config/crd/bases/runtime.cluster.x-k8s.io_extensionconfigs.yaml index dd4f456521d5..4a063b488b11 100644 --- a/config/crd/bases/runtime.cluster.x-k8s.io_extensionconfigs.yaml +++ b/config/crd/bases/runtime.cluster.x-k8s.io_extensionconfigs.yaml @@ -85,8 +85,7 @@ spec: of `url` or `service` must be specified. \n The `host` should not refer to a service running in the cluster; use the `service` field instead. \n The scheme should be \"https\"; the URL should - begin with \"https://\". \"http\" is supported for insecure - development purposes only. \n A path is optional, and if present + begin with \"https://\". \n A path is optional, and if present may be any string permissible in a URL. If a path is set it will be used as prefix and the hook-specific path will be appended. \n Attempting to use a user or basic auth e.g. \"user:password@\" diff --git a/exp/runtime/api/v1alpha1/extensionconfig_types.go b/exp/runtime/api/v1alpha1/extensionconfig_types.go index c1f078c9067f..d5644da22474 100644 --- a/exp/runtime/api/v1alpha1/extensionconfig_types.go +++ b/exp/runtime/api/v1alpha1/extensionconfig_types.go @@ -47,7 +47,6 @@ type ClientConfig struct { // the `service` field instead. // // The scheme should be "https"; the URL should begin with "https://". - // "http" is supported for insecure development purposes only. // // A path is optional, and if present may be any string permissible in // a URL. If a path is set it will be used as prefix and the hook-specific diff --git a/exp/runtime/internal/controllers/extensionconfig_controller_test.go b/exp/runtime/internal/controllers/extensionconfig_controller_test.go index d4cb1ecd202c..b9ef308c6127 100644 --- a/exp/runtime/internal/controllers/extensionconfig_controller_test.go +++ b/exp/runtime/internal/controllers/extensionconfig_controller_test.go @@ -220,6 +220,7 @@ func TestExtensionReconciler_discoverExtensionConfig(t *testing.T) { }) extensionConfig := fakeExtensionConfigForURL(ns.Name, extensionName, srv1.URL) + extensionConfig.Spec.ClientConfig.CABundle = testcerts.CACert discoveredExtensionConfig, err := discoverExtensionConfig(ctx, runtimeClient, extensionConfig) g.Expect(err).To(BeNil()) @@ -252,7 +253,8 @@ func TestExtensionReconciler_discoverExtensionConfig(t *testing.T) { Registry: registry, }) - extensionConfig := fakeExtensionConfigForURL(ns.Name, extensionName, "http://localhost:31239") + extensionConfig := fakeExtensionConfigForURL(ns.Name, extensionName, "https://localhost:31239") + extensionConfig.Spec.ClientConfig.CABundle = testcerts.CACert discoveredExtensionConfig, err := discoverExtensionConfig(ctx, runtimeClient, extensionConfig) g.Expect(err).ToNot(BeNil()) diff --git a/exp/runtime/internal/controllers/warmup_test.go b/exp/runtime/internal/controllers/warmup_test.go index 5e9989f8199e..f31393dca378 100644 --- a/exp/runtime/internal/controllers/warmup_test.go +++ b/exp/runtime/internal/controllers/warmup_test.go @@ -123,7 +123,7 @@ func Test_warmupRunnable_Start(t *testing.T) { brokenExtension := "ext2" for _, name := range []string{"ext1", "ext2", "ext3"} { if name == brokenExtension { - g.Expect(env.CreateAndWait(ctx, fakeExtensionConfigForURL(ns.Name, name, "http://localhost:1234"))).To(Succeed()) + g.Expect(env.CreateAndWait(ctx, fakeExtensionConfigForURL(ns.Name, name, "https://localhost:1234"))).To(Succeed()) continue } server, err := fakeSecureExtensionServer(discoveryHandler("first", "second", "third")) diff --git a/internal/runtime/client/client.go b/internal/runtime/client/client.go index a7f25ebea030..10a3f798c1a1 100644 --- a/internal/runtime/client/client.go +++ b/internal/runtime/client/client.go @@ -432,21 +432,20 @@ func httpCall(ctx context.Context, request, response runtime.Object, opts *httpC // use client-go's transport.TLSConfigureFor to ensure good defaults for tls client := http.DefaultClient - if opts.config.CABundle != nil { - tlsConfig, err := transport.TLSConfigFor(&transport.Config{ - TLS: transport.TLSConfig{ - CAData: opts.config.CABundle, - ServerName: extensionURL.Hostname(), - }, - }) - if err != nil { - return errors.Wrap(err, "failed to create tls config") - } - // this also adds http2 - client.Transport = utilnet.SetTransportDefaults(&http.Transport{ - TLSClientConfig: tlsConfig, - }) + tlsConfig, err := transport.TLSConfigFor(&transport.Config{ + TLS: transport.TLSConfig{ + CAData: opts.config.CABundle, + ServerName: extensionURL.Hostname(), + }, + }) + if err != nil { + return errors.Wrap(err, "failed to create tls config") } + // this also adds http2 + client.Transport = utilnet.SetTransportDefaults(&http.Transport{ + TLSClientConfig: tlsConfig, + }) + resp, err := client.Do(httpRequest) if err != nil { return errCallingExtensionHandler( @@ -486,12 +485,8 @@ func urlForExtension(config runtimev1.ClientConfig, gvh runtimecatalog.GroupVers if svc.Port != nil { host = net.JoinHostPort(host, strconv.Itoa(int(*svc.Port))) } - scheme := "http" - if len(config.CABundle) > 0 { - scheme = "https" - } u = &url.URL{ - Scheme: scheme, + Scheme: "https", Host: host, } if svc.Path != nil { @@ -506,6 +501,9 @@ func urlForExtension(config runtimev1.ClientConfig, gvh runtimecatalog.GroupVers if err != nil { return nil, errors.Wrap(err, "URL in ClientConfig is invalid") } + if u.Scheme != "https" { + return nil, errors.Errorf("expected https scheme, got %s", u.Scheme) + } } // Add the subpatch to the ExtensionHandler for the given hook. u.Path = path.Join(u.Path, runtimecatalog.GVHToPath(gvh, name)) diff --git a/internal/runtime/client/client_test.go b/internal/runtime/client/client_test.go index 1848c0142574..589742d21de8 100644 --- a/internal/runtime/client/client_test.go +++ b/internal/runtime/client/client_test.go @@ -18,6 +18,7 @@ package client import ( "context" + "crypto/tls" "encoding/json" "fmt" "net" @@ -32,6 +33,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts" "k8s.io/utils/pointer" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -187,11 +189,14 @@ func TestClient_httpCall(t *testing.T) { // create http server with fakeHookHandler mux := http.NewServeMux() mux.HandleFunc("/", fakeHookHandler) - srv := httptest.NewServer(mux) + + srv := newUnstartedTLSServer(mux) + srv.StartTLS() defer srv.Close() // set url to srv for in tt.opts tt.opts.config.URL = pointer.String(srv.URL) + tt.opts.config.CABundle = testcerts.CACert } err := httpCall(context.TODO(), tt.request, tt.response, tt.opts) @@ -260,7 +265,7 @@ func TestURLForExtension(t *testing.T) { extensionHandlerName: "test-handler", }, want: want{ - scheme: "http", + scheme: "https", host: "extension-service.test1.svc:8443", path: runtimecatalog.GVHToPath(gvh, "test-handler"), }, @@ -525,7 +530,8 @@ func TestClient_CallExtension(t *testing.T) { validExtensionHandlerWithFailPolicy := runtimev1.ExtensionConfig{ Spec: runtimev1.ExtensionConfigSpec{ ClientConfig: runtimev1.ClientConfig{ - URL: pointer.String(fmt.Sprintf("http://%s/", testHostPort)), + URL: pointer.String(fmt.Sprintf("https://%s/", testHostPort)), + CABundle: testcerts.CACert, }, NamespaceSelector: &metav1.LabelSelector{}, }, @@ -546,7 +552,8 @@ func TestClient_CallExtension(t *testing.T) { validExtensionHandlerWithIgnorePolicy := runtimev1.ExtensionConfig{ Spec: runtimev1.ExtensionConfigSpec{ ClientConfig: runtimev1.ClientConfig{ - URL: pointer.String(fmt.Sprintf("http://%s/", testHostPort)), + URL: pointer.String(fmt.Sprintf("https://%s/", testHostPort)), + CABundle: testcerts.CACert, }, NamespaceSelector: &metav1.LabelSelector{}}, Status: runtimev1.ExtensionConfigStatus{ @@ -722,8 +729,8 @@ func TestClient_CallExtension(t *testing.T) { if tt.testServer.hostPort == "" { tt.testServer.hostPort = testHostPort } - srv := createTestServer(g, tt.testServer) - srv.Start() + srv := createSecureTestServer(g, tt.testServer) + srv.StartTLS() defer srv.Close() } @@ -773,7 +780,8 @@ func TestClient_CallAllExtensions(t *testing.T) { extensionConfig := runtimev1.ExtensionConfig{ Spec: runtimev1.ExtensionConfigSpec{ ClientConfig: runtimev1.ClientConfig{ - URL: pointer.String(fmt.Sprintf("http://%s/", testHostPort)), + URL: pointer.String(fmt.Sprintf("https://%s/", testHostPort)), + CABundle: testcerts.CACert, }, NamespaceSelector: &metav1.LabelSelector{}, }, @@ -925,8 +933,8 @@ func TestClient_CallAllExtensions(t *testing.T) { if tt.testServer.hostPort == "" { tt.testServer.hostPort = testHostPort } - srv := createTestServer(g, tt.testServer) - srv.Start() + srv := createSecureTestServer(g, tt.testServer) + srv.StartTLS() defer srv.Close() } @@ -1142,7 +1150,7 @@ func response(status runtimehooksv1.ResponseStatus) testServerResponse { } } -func createTestServer(g *WithT, server testServerConfig) *httptest.Server { +func createSecureTestServer(g *WithT, server testServerConfig) *httptest.Server { l, err := net.Listen("tcp", server.hostPort) g.Expect(err).NotTo(HaveOccurred()) @@ -1166,7 +1174,9 @@ func createTestServer(g *WithT, server testServerConfig) *httptest.Server { // Otherwise write a 404. w.WriteHeader(http.StatusNotFound) }) - srv := httptest.NewUnstartedServer(mux) + + srv := newUnstartedTLSServer(mux) + // NewUnstartedServer creates a listener. Close that listener and replace // with the one we created. g.Expect(srv.Listener.Close()).To(Succeed()) @@ -1214,3 +1224,16 @@ func fakeRetryableSuccessResponse(retryAfterSeconds int32) *fakev1alpha1.Retryab }, } } + +func newUnstartedTLSServer(handler http.Handler) *httptest.Server { + cert, err := tls.X509KeyPair(testcerts.ServerCert, testcerts.ServerKey) + if err != nil { + panic(err) + } + srv := httptest.NewUnstartedServer(handler) + srv.TLS = &tls.Config{ + MinVersion: tls.VersionTLS13, + Certificates: []tls.Certificate{cert}, + } + return srv +} diff --git a/internal/webhooks/runtime/extensionconfig_webhook.go b/internal/webhooks/runtime/extensionconfig_webhook.go index 23a665b1ce80..3e2a65e8581a 100644 --- a/internal/webhooks/runtime/extensionconfig_webhook.go +++ b/internal/webhooks/runtime/extensionconfig_webhook.go @@ -143,11 +143,11 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList { *e.Spec.ClientConfig.URL, fmt.Sprintf("must be a valid URL, e.g. https://example.com: %v", err), )) - } else if uri.Scheme != "http" && uri.Scheme != "https" { + } else if uri.Scheme != "https" { allErrs = append(allErrs, field.Invalid( specPath.Child("clientConfig", "url"), *e.Spec.ClientConfig.URL, - "'https' and 'http' are the only allowed URL schemes, e.g. https://example.com", + "'https' is the only allowed URL scheme, e.g. https://example.com", )) } }