From 68feae64060038e099287368f8ecc22195489022 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 30 Jul 2017 14:48:08 -0400 Subject: [PATCH 1/2] Changing from no cert to edge encryption should not panic The new route permission checks were too aggressive, erroring out if the user was attempting to clear route certificates, or panicking if setting a new TLS struct. Fix the conditions to be clearer, and add a test to guard the new edge cases. --- pkg/route/registry/route/strategy.go | 29 ++++++++++++--- pkg/route/registry/route/strategy_test.go | 44 +++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/pkg/route/registry/route/strategy.go b/pkg/route/registry/route/strategy.go index 2a8503517bcf..54282b3ede71 100644 --- a/pkg/route/registry/route/strategy.go +++ b/pkg/route/registry/route/strategy.go @@ -146,24 +146,40 @@ func (s routeStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.O return errs } -func certificateChanged(route, older *routeapi.Route) bool { +func hasCertificateInfo(tls *routeapi.TLSConfig) bool { + if tls == nil { + return false + } + return len(tls.Certificate) > 0 || + len(tls.Key) > 0 || + len(tls.CACertificate) > 0 || + len(tls.DestinationCACertificate) > 0 +} + +func certificateChangeRequiresAuth(route, older *routeapi.Route) bool { switch { case route.Spec.TLS != nil && older.Spec.TLS != nil: a, b := route.Spec.TLS, older.Spec.TLS + if !hasCertificateInfo(a) { + // removing certificate info is allowed + return false + } return a.CACertificate != b.CACertificate || a.Certificate != b.Certificate || a.DestinationCACertificate != b.DestinationCACertificate || a.Key != b.Key - case route.Spec.TLS == nil && older.Spec.TLS == nil: - return false + case route.Spec.TLS != nil: + // using any default certificate is allowed + return hasCertificateInfo(route.Spec.TLS) default: - return true + // all other cases we are not adding additional certificate info + return false } } func (s routeStrategy) validateHostUpdate(ctx apirequest.Context, route, older *routeapi.Route) field.ErrorList { hostChanged := route.Spec.Host != older.Spec.Host - certChanged := certificateChanged(route, older) + certChanged := certificateChangeRequiresAuth(route, older) if !hostChanged && !certChanged { return nil } @@ -191,6 +207,9 @@ func (s routeStrategy) validateHostUpdate(ctx apirequest.Context, route, older * if hostChanged { return kvalidation.ValidateImmutableField(route.Spec.Host, older.Spec.Host, field.NewPath("spec", "host")) } + if route.Spec.TLS == nil || older.Spec.TLS == nil { + return kvalidation.ValidateImmutableField(route.Spec.TLS, older.Spec.TLS, field.NewPath("spec", "tls")) + } errs := kvalidation.ValidateImmutableField(route.Spec.TLS.CACertificate, older.Spec.TLS.CACertificate, field.NewPath("spec", "tls", "caCertificate")) errs = append(errs, kvalidation.ValidateImmutableField(route.Spec.TLS.Certificate, older.Spec.TLS.Certificate, field.NewPath("spec", "tls", "certificate"))...) errs = append(errs, kvalidation.ValidateImmutableField(route.Spec.TLS.DestinationCACertificate, older.Spec.TLS.DestinationCACertificate, field.NewPath("spec", "tls", "destinationCACertificate"))...) diff --git a/pkg/route/registry/route/strategy_test.go b/pkg/route/registry/route/strategy_test.go index b09862661cf5..d9b15d25269f 100644 --- a/pkg/route/registry/route/strategy_test.go +++ b/pkg/route/registry/route/strategy_test.go @@ -328,6 +328,50 @@ func TestHostWithWildcardPolicies(t *testing.T) { allow: false, errs: 1, }, + { + name: "set-to-edge-changed", + host: "host", + expected: "host", + oldHost: "host", + tls: &routeapi.TLSConfig{Termination: routeapi.TLSTerminationEdge}, + oldTLS: nil, + wildcardPolicy: routeapi.WildcardPolicyNone, + allow: false, + errs: 0, + }, + { + name: "cleared-edge", + host: "host", + expected: "host", + oldHost: "host", + tls: nil, + oldTLS: &routeapi.TLSConfig{Termination: routeapi.TLSTerminationEdge}, + wildcardPolicy: routeapi.WildcardPolicyNone, + allow: false, + errs: 0, + }, + { + name: "removed-certificate", + host: "host", + expected: "host", + oldHost: "host", + tls: &routeapi.TLSConfig{Termination: routeapi.TLSTerminationReencrypt}, + oldTLS: &routeapi.TLSConfig{Termination: routeapi.TLSTerminationReencrypt, Certificate: "a"}, + wildcardPolicy: routeapi.WildcardPolicyNone, + allow: false, + errs: 0, + }, + { + name: "added-certificate-and-fails", + host: "host", + expected: "host", + oldHost: "host", + tls: &routeapi.TLSConfig{Termination: routeapi.TLSTerminationReencrypt, Certificate: "a"}, + oldTLS: nil, + wildcardPolicy: routeapi.WildcardPolicyNone, + allow: false, + errs: 1, + }, } for _, tc := range tests { From c4dd4cf368b6204571faacde702e624b930a5214 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 30 Jul 2017 14:21:49 -0400 Subject: [PATCH 2/2] Skip goversioninfo when it is not installed Release image does not contain this and an upstream bug is blocking regeneration of the images due to bad tito dependencies. Change the ordering of the makefile and the release image to make failures easier to work around. --- Makefile | 2 +- hack/lib/build/binaries.sh | 6 +++++- hack/release.sh | 8 +++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 53db851dd902..69c415d4fd95 100644 --- a/Makefile +++ b/Makefile @@ -240,7 +240,7 @@ clean: # # Example: # make release -official-release: build-rpms build-cross +official-release: build-cross build-rpms hack/build-images.sh .PHONY: official-release diff --git a/hack/lib/build/binaries.sh b/hack/lib/build/binaries.sh index 3d01592d2c7d..99c4eba4ee06 100644 --- a/hack/lib/build/binaries.sh +++ b/hack/lib/build/binaries.sh @@ -240,7 +240,7 @@ os::build::internal::build_binaries() { fi if [[ "$platform" == "windows/amd64" ]]; then - rm ${OS_ROOT}/cmd/oc/oc.syso + rm -f ${OS_ROOT}/cmd/oc/oc.syso fi for test in "${tests[@]:+${tests[@]}}"; do @@ -261,6 +261,10 @@ readonly -f os::build::build_binaries # Generates the .syso file used to add compile-time VERSIONINFO metadata to the # Windows binary. function os::build::generate_windows_versioninfo() { + if ! os::util::find::system_binary "goversioninfo" >/dev/null 2>&1; then + os::log::warning "No system binary 'goversioninfo' found, skipping version info for Windows builds" + return 0 + fi os::build::version::get_vars local major="${OS_GIT_MAJOR}" local minor="${OS_GIT_MINOR%+}" diff --git a/hack/release.sh b/hack/release.sh index 011a1f8b0a05..25c943e41f59 100755 --- a/hack/release.sh +++ b/hack/release.sh @@ -15,10 +15,12 @@ elif [[ "$( git rev-parse "${tag}" )" != "$( git rev-parse HEAD )" ]]; then fi commit="$( git rev-parse ${tag} )" -# Ensure that the build is using the latest release image -docker pull "${OS_BUILD_ENV_IMAGE}" +# Ensure that the build is using the latest release image and base content +if [[ -z "${OS_RELEASE_STALE}" ]]; then + docker pull "${OS_BUILD_ENV_IMAGE}" + hack/build-base-images.sh +fi -hack/build-base-images.sh hack/env OS_GIT_COMMIT="${commit}" make official-release OS_PUSH_ALWAYS=1 OS_PUSH_TAG="${tag}" OS_TAG="" OS_PUSH_LOCAL="1" hack/push-release.sh