Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing from no-cert to edge encryption should not panic #15550

Merged
merged 2 commits into from Jul 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion hack/lib/build/binaries.sh
Expand Up @@ -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
Expand All @@ -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%+}"
Expand Down
8 changes: 5 additions & 3 deletions hack/release.sh
Expand Up @@ -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

Expand Down
29 changes: 24 additions & 5 deletions pkg/route/registry/route/strategy.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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"))...)
Expand Down
44 changes: 44 additions & 0 deletions pkg/route/registry/route/strategy_test.go
Expand Up @@ -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 {
Expand Down