From d00906f44e30a44097be27a660518451b7d1d4c6 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 23 Oct 2018 13:29:34 -0400 Subject: [PATCH 1/3] Fix omitempty/optional indicator on CABundle fields --- pkg/apis/admissionregistration/types.go | 6 +++--- pkg/apis/auditregistration/types.go | 5 ++--- .../k8s.io/api/admissionregistration/v1beta1/types.go | 10 +++++----- .../src/k8s.io/api/auditregistration/v1alpha1/types.go | 9 ++++----- .../kube-aggregator/pkg/apis/apiregistration/types.go | 1 + .../pkg/apis/apiregistration/v1/types.go | 1 + .../pkg/apis/apiregistration/v1beta1/types.go | 1 + 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index 511bdd96d1..70b810abab 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -328,9 +328,9 @@ type WebhookClientConfig struct { // +optional Service *ServiceReference - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // Required. + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. + // +optional CABundle []byte } diff --git a/pkg/apis/auditregistration/types.go b/pkg/apis/auditregistration/types.go index 8362483d21..bc6ede7c3d 100644 --- a/pkg/apis/auditregistration/types.go +++ b/pkg/apis/auditregistration/types.go @@ -173,9 +173,8 @@ type WebhookClientConfig struct { // +optional Service *ServiceReference - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // defaults to the apiservers CA bundle for the endpoint type + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional CABundle []byte } diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go index 0b948ba1df..1703ac7134 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go @@ -282,12 +282,12 @@ type WebhookClientConfig struct { // Port 443 will be used if it is open, otherwise it is an error. // // +optional - Service *ServiceReference `json:"service" protobuf:"bytes,1,opt,name=service"` + Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,1,opt,name=service"` - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // Required. - CABundle []byte `json:"caBundle" protobuf:"bytes,2,opt,name=caBundle"` + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. + // +optional + CABundle []byte `json:"caBundle,omitempty" protobuf:"bytes,2,opt,name=caBundle"` } // ServiceReference holds a reference to Service.legacy.k8s.io diff --git a/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go b/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go index a7ef9d13fc..a27d559a4e 100644 --- a/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go +++ b/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go @@ -169,13 +169,12 @@ type WebhookClientConfig struct { // Port 443 will be used if it is open, otherwise it is an error. // // +optional - Service *ServiceReference `json:"service" protobuf:"bytes,2,opt,name=service"` + Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,2,opt,name=service"` - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // defaults to the apiservers CA bundle for the endpoint type + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional - CABundle []byte `json:"caBundle" protobuf:"bytes,3,opt,name=caBundle"` + CABundle []byte `json:"caBundle,omitempty" protobuf:"bytes,3,opt,name=caBundle"` } // ServiceReference holds a reference to Service.legacy.k8s.io diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/types.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/types.go index 3f04221160..459edfe763 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/types.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/types.go @@ -53,6 +53,7 @@ type APIServiceSpec struct { // This is strongly discouraged. You should use the CABundle instead. InsecureSkipTLSVerify bool // CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional CABundle []byte diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/types.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/types.go index ffaec409cb..171ed303ac 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/types.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/types.go @@ -53,6 +53,7 @@ type APIServiceSpec struct { // This is strongly discouraged. You should use the CABundle instead. InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty" protobuf:"varint,4,opt,name=insecureSkipTLSVerify"` // CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional CABundle []byte `json:"caBundle,omitempty" protobuf:"bytes,5,opt,name=caBundle"` diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go index 0d4ba49eff..a95c5642d9 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go @@ -53,6 +53,7 @@ type APIServiceSpec struct { // This is strongly discouraged. You should use the CABundle instead. InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty" protobuf:"varint,4,opt,name=insecureSkipTLSVerify"` // CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional CABundle []byte `json:"caBundle,omitempty" protobuf:"bytes,5,opt,name=caBundle"` From 921c60d30f35477015bcb88eb33bc160b9ab8c03 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 23 Oct 2018 17:47:37 +0000 Subject: [PATCH 2/3] Generated files --- api/openapi-spec/swagger.json | 11 ++++------- .../admissionregistration.k8s.io_v1beta1.json | 6 +----- .../auditregistration.k8s.io_v1alpha1.json | 6 +----- .../v1beta1/definitions.html | 6 +++--- .../v1alpha1/definitions.html | 6 +++--- .../api/admissionregistration/v1beta1/generated.proto | 6 +++--- .../v1beta1/types_swagger_doc_generated.go | 2 +- .../api/auditregistration/v1alpha1/generated.proto | 5 ++--- .../v1alpha1/types_swagger_doc_generated.go | 2 +- .../pkg/apis/apiregistration/v1/generated.proto | 1 + .../pkg/apis/apiregistration/v1beta1/generated.proto | 1 + 11 files changed, 21 insertions(+), 31 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index a6defd83d5..503ed1be27 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -77370,12 +77370,9 @@ }, "io.k8s.api.admissionregistration.v1beta1.WebhookClientConfig": { "description": "WebhookClientConfig contains the information to make a TLS connection with the webhook", - "required": [ - "caBundle" - ], "properties": { "caBundle": { - "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. Required.", + "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used.", "type": "string", "format": "byte" }, @@ -79979,7 +79976,7 @@ "description": "WebhookClientConfig contains the information to make a connection with the webhook", "properties": { "caBundle": { - "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. defaults to the apiservers CA bundle for the endpoint type", + "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used.", "type": "string", "format": "byte" }, @@ -93505,7 +93502,7 @@ ], "properties": { "caBundle": { - "description": "CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate.", + "description": "CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. If unspecified, system trust roots on the apiserver are used.", "type": "string", "format": "byte" }, @@ -93664,7 +93661,7 @@ ], "properties": { "caBundle": { - "description": "CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate.", + "description": "CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. If unspecified, system trust roots on the apiserver are used.", "type": "string", "format": "byte" }, diff --git a/api/swagger-spec/admissionregistration.k8s.io_v1beta1.json b/api/swagger-spec/admissionregistration.k8s.io_v1beta1.json index 4cd641fc88..80d0aa79c5 100644 --- a/api/swagger-spec/admissionregistration.k8s.io_v1beta1.json +++ b/api/swagger-spec/admissionregistration.k8s.io_v1beta1.json @@ -1860,10 +1860,6 @@ "v1beta1.WebhookClientConfig": { "id": "v1beta1.WebhookClientConfig", "description": "WebhookClientConfig contains the information to make a TLS connection with the webhook", - "required": [ - "service", - "caBundle" - ], "properties": { "url": { "type": "string", @@ -1875,7 +1871,7 @@ }, "caBundle": { "type": "string", - "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. Required." + "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used." } } }, diff --git a/api/swagger-spec/auditregistration.k8s.io_v1alpha1.json b/api/swagger-spec/auditregistration.k8s.io_v1alpha1.json index 64c8229ae5..44634e0542 100644 --- a/api/swagger-spec/auditregistration.k8s.io_v1alpha1.json +++ b/api/swagger-spec/auditregistration.k8s.io_v1alpha1.json @@ -1155,10 +1155,6 @@ "v1alpha1.WebhookClientConfig": { "id": "v1alpha1.WebhookClientConfig", "description": "WebhookClientConfig contains the information to make a connection with the webhook", - "required": [ - "service", - "caBundle" - ], "properties": { "url": { "type": "string", @@ -1170,7 +1166,7 @@ }, "caBundle": { "type": "string", - "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. defaults to the apiservers CA bundle for the endpoint type" + "description": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used." } } }, diff --git a/docs/api-reference/admissionregistration.k8s.io/v1beta1/definitions.html b/docs/api-reference/admissionregistration.k8s.io/v1beta1/definitions.html index 51f3f9ae2b..2b6ea0060b 100755 --- a/docs/api-reference/admissionregistration.k8s.io/v1beta1/definitions.html +++ b/docs/api-reference/admissionregistration.k8s.io/v1beta1/definitions.html @@ -1613,14 +1613,14 @@ Attempting to use a user or basic auth e.g. "user:password@" is not allowed. Fra If the webhook is running within the cluster, then you should use service.

Port 443 will be used if it is open, otherwise it is an error.

-

true

+

false

v1beta1.ServiceReference

caBundle

-

caBundle is a PEM encoded CA bundle which will be used to validate the webhook’s server certificate. Required.

-

true

+

caBundle is a PEM encoded CA bundle which will be used to validate the webhook’s server certificate. If unspecified, system trust roots on the apiserver are used.

+

false

string

diff --git a/docs/api-reference/auditregistration.k8s.io/v1alpha1/definitions.html b/docs/api-reference/auditregistration.k8s.io/v1alpha1/definitions.html index 083c31252e..c2b6993043 100755 --- a/docs/api-reference/auditregistration.k8s.io/v1alpha1/definitions.html +++ b/docs/api-reference/auditregistration.k8s.io/v1alpha1/definitions.html @@ -525,14 +525,14 @@ Attempting to use a user or basic auth e.g. "user:password@" is not allowed. Fra If the webhook is running within the cluster, then you should use service.

Port 443 will be used if it is open, otherwise it is an error.

-

true

+

false

v1alpha1.ServiceReference

caBundle

-

caBundle is a PEM encoded CA bundle which will be used to validate the webhook’s server certificate. defaults to the apiservers CA bundle for the endpoint type

-

true

+

caBundle is a PEM encoded CA bundle which will be used to validate the webhook’s server certificate. If unspecified, system trust roots on the apiserver are used.

+

false

string

diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/generated.proto b/staging/src/k8s.io/api/admissionregistration/v1beta1/generated.proto index 4d55ca878a..2a23a37091 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/admissionregistration/v1beta1/generated.proto @@ -261,9 +261,9 @@ message WebhookClientConfig { // +optional optional ServiceReference service = 1; - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // Required. + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. + // +optional optional bytes caBundle = 2; } diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types_swagger_doc_generated.go index aab917a402..12c209b0b8 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/admissionregistration/v1beta1/types_swagger_doc_generated.go @@ -116,7 +116,7 @@ var map_WebhookClientConfig = map[string]string{ "": "WebhookClientConfig contains the information to make a TLS connection with the webhook", "url": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nThe scheme must be \"https\"; the URL must begin with \"https://\".\n\nA path is optional, and if present may be any string permissible in a URL. You may use the path to pass an arbitrary string to the webhook, for example, a cluster identifier.\n\nAttempting to use a user or basic auth e.g. \"user:password@\" is not allowed. Fragments (\"#...\") and query parameters (\"?...\") are not allowed, either.", "service": "`service` is a reference to the service for this webhook. Either `service` or `url` must be specified.\n\nIf the webhook is running within the cluster, then you should use `service`.\n\nPort 443 will be used if it is open, otherwise it is an error.", - "caBundle": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. Required.", + "caBundle": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used.", } func (WebhookClientConfig) SwaggerDoc() map[string]string { diff --git a/staging/src/k8s.io/api/auditregistration/v1alpha1/generated.proto b/staging/src/k8s.io/api/auditregistration/v1alpha1/generated.proto index ba42a1cf38..1b715f062e 100644 --- a/staging/src/k8s.io/api/auditregistration/v1alpha1/generated.proto +++ b/staging/src/k8s.io/api/auditregistration/v1alpha1/generated.proto @@ -137,9 +137,8 @@ message WebhookClientConfig { // +optional optional ServiceReference service = 2; - // `caBundle` is a PEM encoded CA bundle which will be used to validate - // the webhook's server certificate. - // defaults to the apiservers CA bundle for the endpoint type + // `caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional optional bytes caBundle = 3; } diff --git a/staging/src/k8s.io/api/auditregistration/v1alpha1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/auditregistration/v1alpha1/types_swagger_doc_generated.go index 914932e6aa..0fe9133326 100644 --- a/staging/src/k8s.io/api/auditregistration/v1alpha1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/auditregistration/v1alpha1/types_swagger_doc_generated.go @@ -90,7 +90,7 @@ var map_WebhookClientConfig = map[string]string{ "": "WebhookClientConfig contains the information to make a connection with the webhook", "url": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nThe scheme must be \"https\"; the URL must begin with \"https://\".\n\nA path is optional, and if present may be any string permissible in a URL. You may use the path to pass an arbitrary string to the webhook, for example, a cluster identifier.\n\nAttempting to use a user or basic auth e.g. \"user:password@\" is not allowed. Fragments (\"#...\") and query parameters (\"?...\") are not allowed, either.", "service": "`service` is a reference to the service for this webhook. Either `service` or `url` must be specified.\n\nIf the webhook is running within the cluster, then you should use `service`.\n\nPort 443 will be used if it is open, otherwise it is an error.", - "caBundle": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. defaults to the apiservers CA bundle for the endpoint type", + "caBundle": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used.", } func (WebhookClientConfig) SwaggerDoc() map[string]string { diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/generated.proto b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/generated.proto index 5e24aa5d0e..f33fc31dc7 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/generated.proto +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/generated.proto @@ -88,6 +88,7 @@ message APIServiceSpec { optional bool insecureSkipTLSVerify = 4; // CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional optional bytes caBundle = 5; diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/generated.proto b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/generated.proto index 3a45347a79..d3b30e742b 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/generated.proto +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/generated.proto @@ -88,6 +88,7 @@ message APIServiceSpec { optional bool insecureSkipTLSVerify = 4; // CABundle is a PEM encoded CA bundle which will be used to validate an API server's serving certificate. + // If unspecified, system trust roots on the apiserver are used. // +optional optional bytes caBundle = 5; From fbd5597e9914fc24ece3b21e6015748ed865de7b Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 23 Oct 2018 13:48:05 -0400 Subject: [PATCH 3/3] Add system root unit test --- .../client-go/transport/transport_test.go | 38 +++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/client-go/transport/transport_test.go b/staging/src/k8s.io/client-go/transport/transport_test.go index 2e9896a08b..eead38aacc 100644 --- a/staging/src/k8s.io/client-go/transport/transport_test.go +++ b/staging/src/k8s.io/client-go/transport/transport_test.go @@ -93,20 +93,32 @@ stR0Yiw0buV6DL/moUO0HIM9Bjh96HJp+LxiIS6UCdIhMPp5HoQa func TestNew(t *testing.T) { testCases := map[string]struct { - Config *Config - Err bool - TLS bool - TLSCert bool - TLSErr bool - Default bool + Config *Config + Err bool + TLS bool + TLSCert bool + TLSErr bool + Default bool + Insecure bool + DefaultRoots bool }{ "default transport": { Default: true, Config: &Config{}, }, + "insecure": { + TLS: true, + Insecure: true, + DefaultRoots: true, + Config: &Config{TLS: TLSConfig{ + Insecure: true, + }}, + }, + "server name": { - TLS: true, + TLS: true, + DefaultRoots: true, Config: &Config{TLS: TLSConfig{ ServerName: "foo", }}, @@ -266,6 +278,18 @@ func TestNew(t *testing.T) { return } + switch { + case testCase.DefaultRoots && transport.TLSClientConfig.RootCAs != nil: + t.Fatalf("got %#v, expected nil root CAs", transport.TLSClientConfig.RootCAs) + case !testCase.DefaultRoots && transport.TLSClientConfig.RootCAs == nil: + t.Fatalf("got %#v, expected non-nil root CAs", transport.TLSClientConfig.RootCAs) + } + + switch { + case testCase.Insecure != transport.TLSClientConfig.InsecureSkipVerify: + t.Fatalf("got %#v, expected %#v", transport.TLSClientConfig.InsecureSkipVerify, testCase.Insecure) + } + switch { case testCase.TLSCert && transport.TLSClientConfig.GetClientCertificate == nil: t.Fatalf("got %#v, expected TLSClientConfig.GetClientCertificate", transport.TLSClientConfig)