From 0531c5f86668231d21f3fdfbcfc111512f925ca5 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Tue, 10 Feb 2026 07:46:04 -0300 Subject: [PATCH] remove RouteExternalCertificate feature gate RouteExternalCertificate is now enabled by default. This update is removing the opts used to hold its value, hardcoding the behavior when the value is true. This update needs to be in sync with o/kubernetes and o/openshift-apiserver --- .../config.openshift.io/featuregates.yaml | 1 - .../featuregates/cluster.yaml | 1 - .../featuregates/cluster.yaml | 1 - pkg/route/common.go | 9 ---- pkg/route/hostassignment/assignment.go | 38 +++++++------- pkg/route/hostassignment/assignment_test.go | 51 +------------------ pkg/route/validation/validation.go | 20 ++++---- pkg/route/validation/validation_test.go | 25 +++------ 8 files changed, 35 insertions(+), 111 deletions(-) diff --git a/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates.yaml b/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates.yaml index b940d55fd4..50a8978a9a 100644 --- a/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates.yaml +++ b/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates.yaml @@ -59,7 +59,6 @@ items: - name: MixedCPUsAllocation - name: NetworkLiveMigration - name: NodeSwap - - name: RouteExternalCertificate - name: SigstoreImageVerification - name: VSphereControlPlaneMachineSet - name: VSphereStaticIPs diff --git a/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml b/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml index 1f9f309daf..d53dd2b726 100644 --- a/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml +++ b/pkg/manifestclienttest/testdata/must-gather-01/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml @@ -56,7 +56,6 @@ status: - name: MixedCPUsAllocation - name: NetworkLiveMigration - name: NodeSwap - - name: RouteExternalCertificate - name: SigstoreImageVerification - name: VSphereControlPlaneMachineSet - name: VSphereStaticIPs diff --git a/pkg/manifestclienttest/testdata/no-namespaces/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml b/pkg/manifestclienttest/testdata/no-namespaces/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml index 1f9f309daf..d53dd2b726 100644 --- a/pkg/manifestclienttest/testdata/no-namespaces/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml +++ b/pkg/manifestclienttest/testdata/no-namespaces/cluster-scoped-resources/config.openshift.io/featuregates/cluster.yaml @@ -56,7 +56,6 @@ status: - name: MixedCPUsAllocation - name: NetworkLiveMigration - name: NodeSwap - - name: RouteExternalCertificate - name: SigstoreImageVerification - name: VSphereControlPlaneMachineSet - name: VSphereStaticIPs diff --git a/pkg/route/common.go b/pkg/route/common.go index bd378da01c..8bef4ecb1f 100644 --- a/pkg/route/common.go +++ b/pkg/route/common.go @@ -12,12 +12,3 @@ import ( type SubjectAccessReviewCreator interface { Create(ctx context.Context, sar *authorizationv1.SubjectAccessReview, opts metav1.CreateOptions) (*authorizationv1.SubjectAccessReview, error) } - -// RouteValidationOptions used to tweak how/what fields are validated. These -// options are propagated by the apiserver. -type RouteValidationOptions struct { - - // AllowExternalCertificates option is set when the RouteExternalCertificate - // feature gate is enabled. - AllowExternalCertificates bool -} diff --git a/pkg/route/hostassignment/assignment.go b/pkg/route/hostassignment/assignment.go index 81cd5a656d..bed18cf0dc 100644 --- a/pkg/route/hostassignment/assignment.go +++ b/pkg/route/hostassignment/assignment.go @@ -37,7 +37,7 @@ type HostnameGenerator interface { // AllocateHost allocates a host name ONLY if the route doesn't specify a subdomain wildcard policy and // the host name on the route is empty and an allocator is configured. // It must first allocate the shard and may return an error if shard allocation fails. -func AllocateHost(ctx context.Context, route *routev1.Route, sarc route.SubjectAccessReviewCreator, routeAllocator HostnameGenerator, opts route.RouteValidationOptions) field.ErrorList { +func AllocateHost(ctx context.Context, route *routev1.Route, sarc route.SubjectAccessReviewCreator, routeAllocator HostnameGenerator) field.ErrorList { hostSet := len(route.Spec.Host) > 0 certSet := route.Spec.TLS != nil && (len(route.Spec.TLS.CACertificate) > 0 || @@ -45,7 +45,7 @@ func AllocateHost(ctx context.Context, route *routev1.Route, sarc route.SubjectA len(route.Spec.TLS.DestinationCACertificate) > 0 || len(route.Spec.TLS.Key) > 0) - if opts.AllowExternalCertificates && route.Spec.TLS != nil && route.Spec.TLS.ExternalCertificate != nil { + if route.Spec.TLS != nil && route.Spec.TLS.ExternalCertificate != nil { certSet = certSet || len(route.Spec.TLS.ExternalCertificate.Name) > 0 } @@ -103,7 +103,7 @@ func AllocateHost(ctx context.Context, route *routev1.Route, sarc route.SubjectA return nil } -func hasCertificateInfo(tls *routev1.TLSConfig, opts route.RouteValidationOptions) bool { +func hasCertificateInfo(tls *routev1.TLSConfig) bool { if tls == nil { return false } @@ -112,7 +112,7 @@ func hasCertificateInfo(tls *routev1.TLSConfig, opts route.RouteValidationOption len(tls.CACertificate) > 0 || len(tls.DestinationCACertificate) > 0 - if opts.AllowExternalCertificates && tls.ExternalCertificate != nil { + if tls.ExternalCertificate != nil { hasInfo = hasInfo || len(tls.ExternalCertificate.Name) > 0 } return hasInfo @@ -122,11 +122,11 @@ func hasCertificateInfo(tls *routev1.TLSConfig, opts route.RouteValidationOption // Note: If (newer/updated) route uses externalCertificate, this function always returns true, as we cannot definitively verify if // the content of the referenced secret has been modified. Even if the secret name remains the same, // we must assume that the secret content is changed, necessitating authorization. -func certificateChangeRequiresAuth(route, older *routev1.Route, opts route.RouteValidationOptions) bool { +func certificateChangeRequiresAuth(route, older *routev1.Route) bool { switch { case route.Spec.TLS != nil && older.Spec.TLS != nil: a, b := route.Spec.TLS, older.Spec.TLS - if !hasCertificateInfo(a, opts) { + if !hasCertificateInfo(a) { // removing certificate info is allowed return false } @@ -136,16 +136,14 @@ func certificateChangeRequiresAuth(route, older *routev1.Route, opts route.Route a.DestinationCACertificate != b.DestinationCACertificate || a.Key != b.Key - if opts.AllowExternalCertificates { - if route.Spec.TLS.ExternalCertificate != nil { - certChanged = true - } + if route.Spec.TLS.ExternalCertificate != nil { + certChanged = true } return certChanged case route.Spec.TLS != nil: // using any default certificate is allowed - return hasCertificateInfo(route.Spec.TLS, opts) + return hasCertificateInfo(route.Spec.TLS) default: // all other cases we are not adding additional certificate info return false @@ -177,10 +175,10 @@ func validateImmutableField(newVal, oldVal interface{}, fldPath *field.Path, err // since we cannot verify state of external secret object. // Due to this it proceeds with the assumption that the certificate has changed // when the route has externalCertificate set. -func ValidateHostUpdate(ctx context.Context, route, older *routev1.Route, sarc route.SubjectAccessReviewCreator, opts route.RouteValidationOptions) field.ErrorList { +func ValidateHostUpdate(ctx context.Context, route, older *routev1.Route, sarc route.SubjectAccessReviewCreator) field.ErrorList { hostChanged := route.Spec.Host != older.Spec.Host subdomainChanged := route.Spec.Subdomain != older.Spec.Subdomain - certChanged := certificateChangeRequiresAuth(route, older, opts) + certChanged := certificateChangeRequiresAuth(route, older) if !hostChanged && !certChanged && !subdomainChanged { return nil } @@ -251,14 +249,12 @@ func ValidateHostUpdate(ctx context.Context, route, older *routev1.Route, sarc r errs = append(errs, validateImmutableField(route.Spec.TLS.DestinationCACertificate, older.Spec.TLS.DestinationCACertificate, field.NewPath("spec", "tls", "destinationCACertificate"), routeTLSPermissionErrMsg)...) errs = append(errs, validateImmutableField(route.Spec.TLS.Key, older.Spec.TLS.Key, field.NewPath("spec", "tls", "key"), routeTLSPermissionErrMsg)...) - if opts.AllowExternalCertificates { - if route.Spec.TLS.ExternalCertificate == nil || older.Spec.TLS.ExternalCertificate == nil { - errs = append(errs, validateImmutableField(route.Spec.TLS.ExternalCertificate, older.Spec.TLS.ExternalCertificate, field.NewPath("spec", "tls", "externalCertificate"), routeTLSPermissionErrMsg)...) - } else { - // since the state of the external secret cannot be verified, return error (even when secret name remains unchanged) - // without performing immutability checks, if externalCertificate is set. - errs = append(errs, field.Invalid(field.NewPath("spec", "tls", "externalCertificate"), route.Spec.TLS.ExternalCertificate, routeTLSPermissionErrMsg)) - } + if route.Spec.TLS.ExternalCertificate == nil || older.Spec.TLS.ExternalCertificate == nil { + errs = append(errs, validateImmutableField(route.Spec.TLS.ExternalCertificate, older.Spec.TLS.ExternalCertificate, field.NewPath("spec", "tls", "externalCertificate"), routeTLSPermissionErrMsg)...) + } else { + // since the state of the external secret cannot be verified, return error (even when secret name remains unchanged) + // without performing immutability checks, if externalCertificate is set. + errs = append(errs, field.Invalid(field.NewPath("spec", "tls", "externalCertificate"), route.Spec.TLS.ExternalCertificate, routeTLSPermissionErrMsg)) } return errs } diff --git a/pkg/route/hostassignment/assignment_test.go b/pkg/route/hostassignment/assignment_test.go index a0062c4f0d..9f1d6afbc1 100644 --- a/pkg/route/hostassignment/assignment_test.go +++ b/pkg/route/hostassignment/assignment_test.go @@ -12,7 +12,6 @@ import ( "k8s.io/apiserver/pkg/endpoints/request" routev1 "github.com/openshift/api/route/v1" - "github.com/openshift/library-go/pkg/route" ) type testAllocator struct { @@ -53,9 +52,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { expected string expectedSubdomain string - // field for externalCertificate - opts route.RouteValidationOptions - errs int allow bool }{ @@ -139,8 +135,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 1, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "tls-permission-allowed-external-certificate", @@ -149,8 +143,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "no-host-but-allowed", @@ -293,7 +285,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: false, errs: 1, // AllocateHost() -> routeHostPermissionErrMsg - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "create-with-external-certificate-allowed", @@ -303,8 +294,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-added-from-no-certificate-denied", @@ -316,8 +305,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 1, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-added-from-no-certificate-allowed", @@ -329,8 +316,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-added-from-nil-tls-denied", @@ -342,8 +327,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 1, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-added-from-nil-tls-allowed", @@ -355,8 +338,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-removed-and-set-no-certificate-denied", @@ -369,8 +350,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: false, // removing certificate info is allowed errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-removed-and-set-no-certificate-allowed", @@ -383,8 +362,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: true, // removing certificate info is allowed errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-removed-and-set-nil-tls-denied", @@ -397,8 +374,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: false, // removing certificate info is allowed errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-removed-and-set-nil-tls-allowed", @@ -411,8 +386,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: true, // removing certificate info is allowed errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-changed-to-certificate-denied", @@ -424,8 +397,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 2, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-changed-to-certificate-allowed", @@ -437,8 +408,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "certificate-changed-to-external-certificate-denied", @@ -450,8 +419,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 2, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "certificate-changed-to-external-certificate-allowed", @@ -463,8 +430,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "certificate-changed-to-external-certificate-denied-and-featuregate-is-not-set", @@ -478,8 +443,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: false}, }, { name: "certificate-changed-to-external-certificate-allowed-but-featuregate-is-not-set", @@ -493,8 +456,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: false}, }, { name: "external-certificate-changed-denied", @@ -506,8 +467,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: false, errs: 1, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-changed-allowed", @@ -519,8 +478,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-unchanged-denied", @@ -533,8 +490,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: false, // permission is required even if referenced secret name remains unchanged errs: 1, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "external-certificate-unchanged-allowed", @@ -546,8 +501,6 @@ func TestHostWithWildcardPolicies(t *testing.T) { wildcardPolicy: routev1.WildcardPolicyNone, allow: true, errs: 0, - - opts: route.RouteValidationOptions{AllowExternalCertificates: true}, }, { name: "ca-certificate-unchanged", @@ -702,9 +655,9 @@ func TestHostWithWildcardPolicies(t *testing.T) { }, }, } - errs = ValidateHostUpdate(ctx, route, oldRoute, &testSAR{allow: tc.allow}, tc.opts) + errs = ValidateHostUpdate(ctx, route, oldRoute, &testSAR{allow: tc.allow}) } else { - errs = AllocateHost(ctx, route, &testSAR{allow: tc.allow}, testAllocator{}, tc.opts) + errs = AllocateHost(ctx, route, &testSAR{allow: tc.allow}, testAllocator{}) } if route.Spec.Host != tc.expected { diff --git a/pkg/route/validation/validation.go b/pkg/route/validation/validation.go index 55f34fe755..4169d4d0b5 100644 --- a/pkg/route/validation/validation.go +++ b/pkg/route/validation/validation.go @@ -76,11 +76,11 @@ var ( permittedResponseHeaderValueRE = regexp.MustCompile(strings.Replace(permittedHeaderValueTemplate, "XYZ", "res", 1)) ) -func ValidateRoute(ctx context.Context, route *routev1.Route, sarCreator routecommon.SubjectAccessReviewCreator, secretsGetter corev1client.SecretsGetter, opts routecommon.RouteValidationOptions) field.ErrorList { +func ValidateRoute(ctx context.Context, route *routev1.Route, sarCreator routecommon.SubjectAccessReviewCreator, secretsGetter corev1client.SecretsGetter) field.ErrorList { allErrs := field.ErrorList{} pathFieldPath := field.NewPath("spec", "path") allErrs = append(allErrs, validatePath(route.Spec.Path, pathFieldPath)...) - allErrs = append(allErrs, validateRoute(ctx, route, true, sarCreator, secretsGetter, opts)...) + allErrs = append(allErrs, validateRoute(ctx, route, true, sarCreator, secretsGetter)...) return allErrs } @@ -105,7 +105,7 @@ func checkLabelSegments(host string) bool { } // validateRoute - private function to validate route -func validateRoute(ctx context.Context, route *routev1.Route, checkHostname bool, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter, opts routecommon.RouteValidationOptions) field.ErrorList { +func validateRoute(ctx context.Context, route *routev1.Route, checkHostname bool, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter) field.ErrorList { //ensure meta is set properly result := validateObjectMeta(&route.ObjectMeta, true, validateRouteName, field.NewPath("metadata")) @@ -212,7 +212,7 @@ func validateRoute(ctx context.Context, route *routev1.Route, checkHostname bool } } - if errs := validateTLS(ctx, route, specPath.Child("tls"), sarc, secrets, opts); len(errs) != 0 { + if errs := validateTLS(ctx, route, specPath.Child("tls"), sarc, secrets); len(errs) != 0 { result = append(result, errs...) } @@ -233,11 +233,11 @@ func validatePath(pathValue string, fldPath *field.Path) field.ErrorList { return result } -func ValidateRouteUpdate(ctx context.Context, route *routev1.Route, older *routev1.Route, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter, opts routecommon.RouteValidationOptions) field.ErrorList { +func ValidateRouteUpdate(ctx context.Context, route *routev1.Route, older *routev1.Route, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter) field.ErrorList { allErrs := validateObjectMetaUpdate(&route.ObjectMeta, &older.ObjectMeta, field.NewPath("metadata")) allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(route.Spec.WildcardPolicy, older.Spec.WildcardPolicy, field.NewPath("spec", "wildcardPolicy"))...) hostnameUpdated := route.Spec.Host != older.Spec.Host - allErrs = append(allErrs, validateRoute(ctx, route, hostnameUpdated && validLabels(older.Spec.Host), sarc, secrets, opts)...) + allErrs = append(allErrs, validateRoute(ctx, route, hostnameUpdated && validLabels(older.Spec.Host), sarc, secrets)...) if route.Spec.Path != older.Spec.Path { pathFieldPath := field.NewPath("spec", "path") allErrs = append(allErrs, validatePath(route.Spec.Path, pathFieldPath)...) @@ -258,7 +258,7 @@ func ValidateRouteStatusUpdate(route *routev1.Route, older *routev1.Route) field // validateTLS tests fields for different types of TLS combinations are set. Called // by ValidateRoute. -func validateTLS(ctx context.Context, route *routev1.Route, fldPath *field.Path, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter, opts routecommon.RouteValidationOptions) field.ErrorList { +func validateTLS(ctx context.Context, route *routev1.Route, fldPath *field.Path, sarc routecommon.SubjectAccessReviewCreator, secrets corev1client.SecretsGetter) field.ErrorList { result := field.ErrorList{} tls := route.Spec.TLS @@ -272,7 +272,7 @@ func validateTLS(ctx context.Context, route *routev1.Route, fldPath *field.Path, // reencrypt may specify destination ca cert // externalCert, cert, key, cacert may not be specified because the route may be a wildcard case routev1.TLSTerminationReencrypt: - if opts.AllowExternalCertificates && tls.ExternalCertificate != nil { + if tls.ExternalCertificate != nil { if len(tls.Certificate) > 0 && len(tls.ExternalCertificate.Name) > 0 { result = append(result, field.Invalid(fldPath.Child("externalCertificate"), tls.ExternalCertificate.Name, "cannot specify both tls.certificate and tls.externalCertificate")) } else if len(tls.ExternalCertificate.Name) > 0 { @@ -290,7 +290,7 @@ func validateTLS(ctx context.Context, route *routev1.Route, fldPath *field.Path, result = append(result, field.Invalid(fldPath.Child("key"), "redacted key data", "passthrough termination does not support certificates")) } - if opts.AllowExternalCertificates && tls.ExternalCertificate != nil { + if tls.ExternalCertificate != nil { if len(tls.ExternalCertificate.Name) > 0 { result = append(result, field.Invalid(fldPath.Child("externalCertificate"), tls.ExternalCertificate.Name, "passthrough termination does not support certificates")) } @@ -310,7 +310,7 @@ func validateTLS(ctx context.Context, route *routev1.Route, fldPath *field.Path, result = append(result, field.Invalid(fldPath.Child("destinationCACertificate"), "redacted destination ca certificate data", "edge termination does not support destination certificates")) } - if opts.AllowExternalCertificates && tls.ExternalCertificate != nil { + if tls.ExternalCertificate != nil { if len(tls.Certificate) > 0 && len(tls.ExternalCertificate.Name) > 0 { result = append(result, field.Invalid(fldPath.Child("externalCertificate"), tls.ExternalCertificate.Name, "cannot specify both tls.certificate and tls.externalCertificate")) } else if len(tls.ExternalCertificate.Name) > 0 { diff --git a/pkg/route/validation/validation_test.go b/pkg/route/validation/validation_test.go index d2f05c3f1c..ee048c2789 100644 --- a/pkg/route/validation/validation_test.go +++ b/pkg/route/validation/validation_test.go @@ -19,7 +19,6 @@ import ( corev1client "k8s.io/client-go/kubernetes/typed/core/v1" routev1 "github.com/openshift/api/route/v1" - routecommon "github.com/openshift/library-go/pkg/route" ) const ( @@ -1042,7 +1041,7 @@ func TestValidateRoute(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - errs := ValidateRoute(context.Background(), tc.route, &testSARCreator{allow: false}, &testSecretGetter{}, routecommon.RouteValidationOptions{AllowExternalCertificates: false}) + errs := ValidateRoute(context.Background(), tc.route, &testSARCreator{allow: false}, &testSecretGetter{}) if len(errs) != tc.expectedErrors { t.Errorf("Test case %s expected %d error(s), got %d. %v", tc.name, tc.expectedErrors, len(errs), errs) } @@ -1876,7 +1875,6 @@ func TestValidateTLS(t *testing.T) { // fields for externalCertificate allow bool secret *corev1.Secret - opts routecommon.RouteValidationOptions }{ { name: "No TLS Termination", @@ -2039,7 +2037,6 @@ func TestValidateTLS(t *testing.T) { }, }, }, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2055,7 +2052,6 @@ func TestValidateTLS(t *testing.T) { }, }, }, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2070,7 +2066,6 @@ func TestValidateTLS(t *testing.T) { }, }, }, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2097,7 +2092,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: false, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 3, }, { @@ -2124,7 +2118,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: false, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 3, }, { @@ -2151,7 +2144,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2178,7 +2170,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2205,7 +2196,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeBasicAuth, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2232,7 +2222,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeBasicAuth, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 1, }, { @@ -2259,7 +2248,6 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 0, }, { @@ -2286,14 +2274,13 @@ func TestValidateTLS(t *testing.T) { Type: corev1.SecretTypeTLS, }, allow: true, - opts: routecommon.RouteValidationOptions{AllowExternalCertificates: true}, expectedErrors: 0, }, } ctx := request.WithUser(context.Background(), &user.DefaultInfo{}) for _, tc := range tests { - errs := validateTLS(ctx, tc.route, nil, &testSARCreator{allow: tc.allow}, &testSecretGetter{namespace: tc.route.Namespace, secret: tc.secret}, tc.opts) + errs := validateTLS(ctx, tc.route, nil, &testSARCreator{allow: tc.allow}, &testSecretGetter{namespace: tc.route.Namespace, secret: tc.secret}) if len(errs) != tc.expectedErrors { t.Errorf("Test case %s expected %d error(s), got %d. %v", tc.name, tc.expectedErrors, len(errs), errs) } @@ -2321,7 +2308,7 @@ func TestValidatePassthroughInsecureEdgeTerminationPolicy(t *testing.T) { }, } route.Spec.TLS.InsecureEdgeTerminationPolicy = key - errs := validateTLS(context.Background(), route, nil, &testSARCreator{allow: false}, &testSecretGetter{}, routecommon.RouteValidationOptions{}) + errs := validateTLS(context.Background(), route, nil, &testSARCreator{allow: false}, &testSecretGetter{}) if !expected && len(errs) != 0 { t.Errorf("Test case for Passthrough termination with insecure=%s got %d errors where none where expected. %v", key, len(errs), errs) @@ -2591,7 +2578,7 @@ func TestValidateRouteUpdate(t *testing.T) { for i, tc := range tests { newRoute := tc.route.DeepCopy() tc.change(newRoute) - errs := ValidateRouteUpdate(context.Background(), newRoute, tc.route, &testSARCreator{allow: false}, &testSecretGetter{}, routecommon.RouteValidationOptions{}) + errs := ValidateRouteUpdate(context.Background(), newRoute, tc.route, &testSARCreator{allow: false}, &testSecretGetter{}) if len(errs) != tc.expectedErrors { t.Errorf("%d: expected %d error(s), got %d. %v", i, tc.expectedErrors, len(errs), errs) } @@ -2767,7 +2754,7 @@ func TestValidateInsecureEdgeTerminationPolicy(t *testing.T) { }, }, } - errs := validateTLS(context.Background(), route, nil, &testSARCreator{allow: false}, &testSecretGetter{}, routecommon.RouteValidationOptions{}) + errs := validateTLS(context.Background(), route, nil, &testSARCreator{allow: false}, &testSecretGetter{}) if len(errs) != tc.expectedErrors { t.Errorf("Test case %s expected %d error(s), got %d. %v", tc.name, tc.expectedErrors, len(errs), errs) @@ -2825,7 +2812,7 @@ func TestValidateEdgeReencryptInsecureEdgeTerminationPolicy(t *testing.T) { for _, tc := range tests { for key, expected := range insecureTypes { tc.route.Spec.TLS.InsecureEdgeTerminationPolicy = key - errs := validateTLS(context.Background(), tc.route, nil, &testSARCreator{allow: false}, &testSecretGetter{}, routecommon.RouteValidationOptions{}) + errs := validateTLS(context.Background(), tc.route, nil, &testSARCreator{allow: false}, &testSecretGetter{}) if !expected && len(errs) != 0 { t.Errorf("Test case %s with insecure=%s got %d errors where none were expected. %v", tc.name, key, len(errs), errs)