From 5bb8222995712ce03f7371dbc6d897b238916e9c Mon Sep 17 00:00:00 2001 From: Dan Mace Date: Tue, 6 Oct 2015 11:30:46 -0400 Subject: [PATCH] Disable v1beta3 in REST API --- assets/app/config.js | 5 +- assets/app/scripts/services/data.js | 14 +++- assets/test/spec/services/dataServiceSpec.js | 8 +- hack/test-cmd.sh | 6 +- hack/util.sh | 2 +- pkg/assets/bindata.go | 10 ++- pkg/assets/handlers.go | 7 +- pkg/client/client.go | 8 +- pkg/cmd/cli/cmd/loginoptions.go | 2 +- pkg/cmd/server/api/types.go | 25 +++++- pkg/cmd/server/api/v1/conversions.go | 2 +- pkg/cmd/server/api/validation/master.go | 32 +++++-- pkg/cmd/server/api/validation/master_test.go | 83 +++++++++++++++++++ pkg/cmd/server/kubernetes/master.go | 8 +- pkg/cmd/server/kubernetes/master_config.go | 3 +- pkg/cmd/server/origin/asset.go | 11 ++- pkg/cmd/server/origin/handlers.go | 3 - pkg/cmd/server/origin/master.go | 16 +--- pkg/cmd/util/clientcmd/errors_test.go | 21 +++++ pkg/cmd/util/clientcmd/factory.go | 50 ++++++++--- pkg/cmd/util/clientcmd/factory_test.go | 71 ++++++++++++++++ test/cmd/basicresources.sh | 5 ++ test/cmd/builds.sh | 6 +- test/fixtures/mixed-api-versions.yaml | 41 +++++++++ test/integration/router/router_http_server.go | 2 - test/integration/template_test.go | 14 ++-- .../v1.0.0/test-end-to-end.sh | 2 +- 27 files changed, 367 insertions(+), 90 deletions(-) create mode 100644 pkg/cmd/util/clientcmd/errors_test.go create mode 100644 pkg/cmd/util/clientcmd/factory_test.go create mode 100644 test/fixtures/mixed-api-versions.yaml diff --git a/assets/app/config.js b/assets/app/config.js index fd0c45a53730..d964b623c009 100644 --- a/assets/app/config.js +++ b/assets/app/config.js @@ -6,8 +6,7 @@ window.OPENSHIFT_CONFIG = { openshift: { hostPort: "localhost:8443", prefixes: { - "v1beta3": "/osapi", - "*": "/oapi" + "v1": "/oapi" }, resources: { "buildconfigs": true, @@ -60,7 +59,7 @@ window.OPENSHIFT_CONFIG = { k8s: { hostPort: "localhost:8443", prefixes: { - "*": "/api" + "v1": "/api" }, resources: { "bindings": true, diff --git a/assets/app/scripts/services/data.js b/assets/app/scripts/services/data.js index 7f171239ce2f..d24af420265f 100644 --- a/assets/app/scripts/services/data.js +++ b/assets/app/scripts/services/data.js @@ -255,9 +255,17 @@ angular.module('openshiftConsole') var resource = self.kindToResource(object.kind); if (!resource) { failureResults.push({ - data: { - message: "Unrecognized kind: " + object.kind + "." - } + data: {message: "Unrecognized kind " + object.kind} + }); + remaining--; + _checkDone(); + return; + } + + var resourceInfo = self.resourceInfo(resource, object.apiVersion); + if (!resourceInfo) { + failureResults.push({ + data: {message: "Unknown API version "+object.apiVersion+" for kind " + object.kind} }); remaining--; _checkDone(); diff --git a/assets/test/spec/services/dataServiceSpec.js b/assets/test/spec/services/dataServiceSpec.js index 3a689b58e5ee..19c428eeccc0 100644 --- a/assets/test/spec/services/dataServiceSpec.js +++ b/assets/test/spec/services/dataServiceSpec.js @@ -59,13 +59,13 @@ describe("DataService", function(){ [{resource:'pods/proxy', name:"mypod", namespace:"myns", myparam1:"myvalue"}, "http://localhost:8443/api/v1/namespaces/myns/pods/mypod/proxy?myparam1=myvalue"], // Different API versions - [{resource:'builds',apiVersion:'v1beta3'}, "http://localhost:8443/osapi/v1beta3/builds"], + [{resource:'builds',apiVersion:'v1beta3'}, null], [{resource:'builds',apiVersion:'v1' }, "http://localhost:8443/oapi/v1/builds"], - [{resource:'builds',apiVersion:'unknown'}, "http://localhost:8443/oapi/unknown/builds"], + [{resource:'builds',apiVersion:'unknown'}, null], - [{resource:'pods', apiVersion:'v1beta3'}, "http://localhost:8443/api/v1beta3/pods"], + [{resource:'pods', apiVersion:'v1beta3'}, null], [{resource:'pods', apiVersion:'v1' }, "http://localhost:8443/api/v1/pods"], - [{resource:'pods', apiVersion:'unknown'}, "http://localhost:8443/api/unknown/pods"] + [{resource:'pods', apiVersion:'unknown'}, null] ]; angular.forEach(tc, function(item) { diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index bd3f85a1ec3a..200aa43eb4d4 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -135,7 +135,7 @@ openshift start \ # breaking the config fails the validation check cp ${MASTER_CONFIG_DIR}/master-config.yaml ${BASETMPDIR}/master-config-broken.yaml os::util::sed '5,10d' ${BASETMPDIR}/master-config-broken.yaml -[ "$(openshift ex validate master-config ${BASETMPDIR}/master-config-broken.yaml 2>&1 | grep error)" ] +[ "$(openshift ex validate master-config ${BASETMPDIR}/master-config-broken.yaml 2>&1 | grep ERROR)" ] cp ${NODE_CONFIG_DIR}/node-config.yaml ${BASETMPDIR}/node-config-broken.yaml os::util::sed '5,10d' ${BASETMPDIR}/node-config-broken.yaml @@ -205,8 +205,8 @@ fi # must only accept one arg (server) [ "$(oc login https://server1 https://server2.com 2>&1 | grep 'Only the server URL may be specified')" ] # logs in with a valid certificate authority -oc login ${KUBERNETES_MASTER} --certificate-authority="${MASTER_CONFIG_DIR}/ca.crt" -u test-user -p anything --api-version=v1beta3 -grep -q "v1beta3" ${HOME}/.kube/config +oc login ${KUBERNETES_MASTER} --certificate-authority="${MASTER_CONFIG_DIR}/ca.crt" -u test-user -p anything --api-version=v1 +grep -q "v1" ${HOME}/.kube/config oc logout # logs in skipping certificate check oc login ${KUBERNETES_MASTER} --insecure-skip-tls-verify -u test-user -p anything diff --git a/hack/util.sh b/hack/util.sh index fea943e780da..1c47abf34359 100644 --- a/hack/util.sh +++ b/hack/util.sh @@ -586,7 +586,7 @@ function os::build:wait_for_start() { function os::build:wait_for_end() { echo "[INFO] Waiting for $1 namespace build to complete" wait_for_command "oc get -n $1 builds | grep -i complete" $((10*TIME_MIN)) "oc get -n $1 builds | grep -i -e failed -e error" - BUILD_ID=`oc get -n $1 builds --output-version=v1beta3 --template="{{with index .items 0}}{{.metadata.name}}{{end}}"` + BUILD_ID=`oc get -n $1 builds --output-version=v1 --template="{{with index .items 0}}{{.metadata.name}}{{end}}"` echo "[INFO] Build ${BUILD_ID} finished" # TODO: fix set +e diff --git a/pkg/assets/bindata.go b/pkg/assets/bindata.go index ddde7c3637da..ceb971354d07 100644 --- a/pkg/assets/bindata.go +++ b/pkg/assets/bindata.go @@ -18537,13 +18537,19 @@ failure:h var f = d.defer(), g = [], h = [], i = this, j = a.length; return a.forEach(function(a) { var d = i.kindToResource(a.kind); -return d ? void i.create(d, null, a, b, c).then(function(a) { +if (!d) return h.push({ +data:{ +message:"Unrecognized kind " + a.kind +} +}), j--, void e(); +var f = i.resourceInfo(d, a.apiVersion); +return f ? void i.create(d, null, a, b, c).then(function(a) { g.push(a), j--, e(); }, function(a) { h.push(a), j--, e(); }) :(h.push({ data:{ -message:"Unrecognized kind: " + a.kind + "." +message:"Unknown API version " + a.apiVersion + " for kind " + a.kind } }), j--, void e()); }), f.promise; diff --git a/pkg/assets/handlers.go b/pkg/assets/handlers.go index df8e53751031..5fed7aabde18 100644 --- a/pkg/assets/handlers.go +++ b/pkg/assets/handlers.go @@ -147,8 +147,7 @@ window.OPENSHIFT_CONFIG = { openshift: { hostPort: "{{ .MasterAddr | js}}", prefixes: { - "v1beta3": "{{ .MasterLegacyPrefix | js}}", - "*": "{{ .MasterPrefix | js}}" + "v1": "{{ .MasterPrefix | js}}" }, resources: { {{range $i,$e := .MasterResources}}{{if $i}}, @@ -158,7 +157,7 @@ window.OPENSHIFT_CONFIG = { k8s: { hostPort: "{{ .KubernetesAddr | js}}", prefixes: { - "*": "{{ .KubernetesPrefix | js}}" + "v1": "{{ .KubernetesPrefix | js}}" }, resources: { {{range $i,$e := .KubernetesResources}}{{if $i}}, @@ -182,8 +181,6 @@ type WebConsoleConfig struct { MasterAddr string // MasterPrefix is the OpenShift API context root MasterPrefix string - // MasterLegacyPrefix is the OpenShift API context root for legacy API versions - MasterLegacyPrefix string // MasterResources holds resource names for the OpenShift API MasterResources []string // KubernetesAddr is the host:port the UI should call the kubernetes API on. Scheme is derived from the scheme the UI is served on, so they must be the same. diff --git a/pkg/client/client.go b/pkg/client/client.go index b352c0d930c4..3705592cc81b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -258,16 +258,10 @@ func SetOpenShiftDefaults(config *kclient.Config) error { } if config.Version == "" { // Clients default to the preferred code API version - // TODO: implement version negotiation (highest version supported by server) config.Version = latest.Version } if config.Prefix == "" { - switch config.Version { - case "v1beta3": - config.Prefix = "/osapi" - default: - config.Prefix = "/oapi" - } + config.Prefix = "/oapi" } version := config.Version versionInterfaces, err := latest.InterfacesFor(version) diff --git a/pkg/cmd/cli/cmd/loginoptions.go b/pkg/cmd/cli/cmd/loginoptions.go index c4ee70604141..0d448f020a21 100644 --- a/pkg/cmd/cli/cmd/loginoptions.go +++ b/pkg/cmd/cli/cmd/loginoptions.go @@ -128,7 +128,7 @@ func (o *LoginOptions) getClientConfig() (*kclient.Config, error) { return nil, err } - result := osClient.Get().AbsPath("/osapi").Do() + result := osClient.Get().AbsPath("/").Do() if result.Error() != nil { switch { case o.InsecureTLS: diff --git a/pkg/cmd/server/api/types.go b/pkg/cmd/server/api/types.go index 7d7e50bdd82d..838c46cf45e1 100644 --- a/pkg/cmd/server/api/types.go +++ b/pkg/cmd/server/api/types.go @@ -4,6 +4,8 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/sets" + + "github.com/openshift/origin/pkg/api/latest" ) // A new entry shall be added to FeatureAliases for every change to following values. @@ -16,10 +18,25 @@ const ( var ( KnownKubernetesAPILevels = []string{"v1beta1", "v1beta2", "v1beta3", "v1"} KnownOpenShiftAPILevels = []string{"v1beta1", "v1beta3", "v1"} - DefaultKubernetesAPILevels = []string{"v1beta3", "v1"} - DefaultOpenShiftAPILevels = []string{"v1beta3", "v1"} - DeadKubernetesAPILevels = []string{"v1beta1", "v1beta2"} - DeadOpenShiftAPILevels = []string{"v1beta1"} + DefaultKubernetesAPILevels = []string{"v1"} + DefaultOpenShiftAPILevels = []string{"v1"} + DeadKubernetesAPILevels = []string{"v1beta1", "v1beta2", "v1beta3"} + DeadOpenShiftAPILevels = []string{"v1beta1", "v1beta3"} + // KnownKubernetesStorageVersionLevels are storage versions that can be + // dealt with internally. + KnownKubernetesStorageVersionLevels = []string{"v1", "v1beta3"} + // KnownOpenShiftStorageVersionLevels are storage versions that can be dealt + // with internally + KnownOpenShiftStorageVersionLevels = latest.Versions + // DefaultOpenShiftStorageVersionLevel is the default storage version for + // resources. + DefaultOpenShiftStorageVersionLevel = latest.Versions[0] + // DeadKubernetesStorageVersionLevels are storage versions which shouldn't + // be exposed externally. + DeadKubernetesStorageVersionLevels = []string{"v1beta3"} + // DeadOpenShiftStorageVersionLevels are storage versions which shouldn't be + // exposed externally. + DeadOpenShiftStorageVersionLevels = []string{"v1beta1", "v1beta3"} // FeatureAliases maps deprecated names of feature flag to their canonical // names. Aliases must be lower-cased for O(1) lookup. diff --git a/pkg/cmd/server/api/v1/conversions.go b/pkg/cmd/server/api/v1/conversions.go index 49da65ccdefb..ae0980e58385 100644 --- a/pkg/cmd/server/api/v1/conversions.go +++ b/pkg/cmd/server/api/v1/conversions.go @@ -96,7 +96,7 @@ func init() { obj.KubernetesStoragePrefix = "kubernetes.io" } if len(obj.OpenShiftStorageVersion) == 0 { - obj.OpenShiftStorageVersion = "v1" + obj.OpenShiftStorageVersion = newer.DefaultOpenShiftStorageVersionLevel } if len(obj.OpenShiftStoragePrefix) == 0 { obj.OpenShiftStoragePrefix = "openshift.io" diff --git a/pkg/cmd/server/api/validation/master.go b/pkg/cmd/server/api/validation/master.go index 1ee9507af889..7f741c821172 100644 --- a/pkg/cmd/server/api/validation/master.go +++ b/pkg/cmd/server/api/validation/master.go @@ -197,12 +197,16 @@ func ValidateAPILevels(apiLevels []string, knownAPILevels, deadAPILevels []strin func ValidateEtcdStorageConfig(config api.EtcdStorageConfig) fielderrors.ValidationErrorList { allErrs := fielderrors.ValidationErrorList{} - if len(config.KubernetesStorageVersion) == 0 { - allErrs = append(allErrs, fielderrors.NewFieldRequired("kubernetesStorageVersion")) - } - if len(config.OpenShiftStorageVersion) == 0 { - allErrs = append(allErrs, fielderrors.NewFieldRequired("openShiftStorageVersion")) - } + allErrs = append(allErrs, ValidateStorageVersionLevel( + config.KubernetesStorageVersion, + api.KnownKubernetesStorageVersionLevels, + api.DeadKubernetesStorageVersionLevels, + "kubernetesStorageVersion")...) + allErrs = append(allErrs, ValidateStorageVersionLevel( + config.OpenShiftStorageVersion, + api.KnownOpenShiftStorageVersionLevels, + api.DeadOpenShiftStorageVersionLevels, + "openShiftStorageVersion")...) if strings.ContainsRune(config.KubernetesStoragePrefix, '%') { allErrs = append(allErrs, fielderrors.NewFieldInvalid("kubernetesStoragePrefix", config.KubernetesStoragePrefix, "the '%' character may not be used in etcd path prefixes")) @@ -214,6 +218,22 @@ func ValidateEtcdStorageConfig(config api.EtcdStorageConfig) fielderrors.Validat return allErrs } +func ValidateStorageVersionLevel(level string, knownAPILevels, deadAPILevels []string, name string) fielderrors.ValidationErrorList { + allErrs := fielderrors.ValidationErrorList{} + + if len(level) == 0 { + allErrs = append(allErrs, fielderrors.NewFieldRequired(name)) + return allErrs + } + supportedLevels := sets.NewString(knownAPILevels...) + supportedLevels.Delete(deadAPILevels...) + if !supportedLevels.Has(level) { + allErrs = append(allErrs, fielderrors.NewFieldValueNotSupported(name, level, supportedLevels.List())) + } + + return allErrs +} + func ValidateServiceAccountConfig(config api.ServiceAccountConfig, builtInKubernetes bool) ValidationResults { validationResults := ValidationResults{} diff --git a/pkg/cmd/server/api/validation/master_test.go b/pkg/cmd/server/api/validation/master_test.go index d7b94d86e00f..54bd74dec45a 100644 --- a/pkg/cmd/server/api/validation/master_test.go +++ b/pkg/cmd/server/api/validation/master_test.go @@ -3,8 +3,11 @@ package validation import ( "testing" + kapi "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/fielderrors" + "github.com/openshift/origin/pkg/cmd/server/api" configapi "github.com/openshift/origin/pkg/cmd/server/api" ) @@ -127,3 +130,83 @@ func TestFailingControllerArgs(t *testing.T) { t.Errorf("expected %v, got %v", e, a) } } + +func TestValidate_ValidateEtcdStorageConfig(t *testing.T) { + osField := "openShiftStorageVersion" + kubeField := "kubernetesStorageVersion" + tests := []struct { + label string + kubeStorageVersion string + openshiftStorageVersion string + name string + expected fielderrors.ValidationErrorList + }{ + { + label: "valid levels", + kubeStorageVersion: "v1", + openshiftStorageVersion: "v1", + expected: fielderrors.ValidationErrorList{}, + }, + { + label: "unknown openshift level", + kubeStorageVersion: "v1", + openshiftStorageVersion: "bogus", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldValueNotSupported(osField, "bogus", []string{"v1"}), + }, + }, + { + label: "unsupported openshift level", + kubeStorageVersion: "v1", + openshiftStorageVersion: "v1beta3", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldValueNotSupported(osField, "v1beta3", []string{"v1"}), + }, + }, + { + label: "missing openshift level", + kubeStorageVersion: "v1", + openshiftStorageVersion: "", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldRequired(osField), + }, + }, + { + label: "unknown kube level", + kubeStorageVersion: "bogus", + openshiftStorageVersion: "v1", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldValueNotSupported(kubeField, "bogus", []string{"v1"}), + }, + }, + { + label: "unsupported kube level", + kubeStorageVersion: "v1beta3", + openshiftStorageVersion: "v1", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldValueNotSupported(kubeField, "v1beta3", []string{"v1"}), + }, + }, + { + label: "missing kube level", + kubeStorageVersion: "", + openshiftStorageVersion: "v1", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldRequired(kubeField), + }, + }, + } + + for _, test := range tests { + t.Logf("evaluating test: %s", test.label) + config := api.EtcdStorageConfig{ + OpenShiftStorageVersion: test.openshiftStorageVersion, + KubernetesStorageVersion: test.kubeStorageVersion, + } + results := ValidateEtcdStorageConfig(config) + if !kapi.Semantic.DeepEqual(test.expected, results) { + t.Errorf("unexpected validation results; diff:\n%v", util.ObjectDiff(test.expected, results)) + return + } + } +} diff --git a/pkg/cmd/server/kubernetes/master.go b/pkg/cmd/server/kubernetes/master.go index 46c683a49780..defa1e4bd52e 100644 --- a/pkg/cmd/server/kubernetes/master.go +++ b/pkg/cmd/server/kubernetes/master.go @@ -31,9 +31,8 @@ import ( ) const ( - KubeAPIPrefix = "/api" - KubeAPIPrefixV1Beta3 = "/api/v1beta3" - KubeAPIPrefixV1 = "/api/v1" + KubeAPIPrefix = "/api" + KubeAPIPrefixV1 = "/api/v1" ) // InstallAPI starts a Kubernetes master and registers the supported REST APIs @@ -45,9 +44,6 @@ func (c *MasterConfig) InstallAPI(container *restful.Container) []string { _ = master.New(c.Master) messages := []string{} - if c.Master.EnableV1Beta3 { - messages = append(messages, fmt.Sprintf("Started Kubernetes API at %%s%s (deprecated)", KubeAPIPrefixV1Beta3)) - } if !c.Master.DisableV1 { messages = append(messages, fmt.Sprintf("Started Kubernetes API at %%s%s", KubeAPIPrefixV1)) } diff --git a/pkg/cmd/server/kubernetes/master_config.go b/pkg/cmd/server/kubernetes/master_config.go index 42030f5ac132..2d6576be4371 100644 --- a/pkg/cmd/server/kubernetes/master_config.go +++ b/pkg/cmd/server/kubernetes/master_config.go @@ -172,8 +172,7 @@ func BuildKubernetesMasterConfig(options configapi.MasterConfig, requestContextM Authorizer: apiserver.NewAlwaysAllowAuthorizer(), AdmissionControl: admissionController, - EnableV1Beta3: configapi.HasKubernetesAPILevel(*options.KubernetesMasterConfig, "v1beta3"), - DisableV1: !configapi.HasKubernetesAPILevel(*options.KubernetesMasterConfig, "v1"), + DisableV1: !configapi.HasKubernetesAPILevel(*options.KubernetesMasterConfig, "v1"), // Set the TLS options for proxying to pods and services // Proxying to nodes uses the kubeletClient TLS config (so can provide a different cert, and verify the node hostname) diff --git a/pkg/cmd/server/origin/asset.go b/pkg/cmd/server/origin/asset.go index decfc4fbff1d..81a4a3ee3b1b 100644 --- a/pkg/cmd/server/origin/asset.go +++ b/pkg/cmd/server/origin/asset.go @@ -164,6 +164,8 @@ func (c *AssetConfig) addHandlers(mux *http.ServeMux) error { versions := sets.NewString() versions.Insert(latest.Versions...) versions.Insert(klatest.Versions...) + deadOriginVersions := sets.NewString(configapi.DeadOpenShiftAPILevels...) + deadKubernetesVersions := sets.NewString(configapi.DeadKubernetesAPILevels...) for _, version := range versions.List() { for kind, t := range api.Scheme.KnownTypes(version) { if strings.Contains(t.PkgPath(), "kubernetes/pkg/expapi") { @@ -174,9 +176,13 @@ func (c *AssetConfig) addHandlers(mux *http.ServeMux) error { } resource, _ := meta.KindToResource(kind, false) if latest.OriginKind(kind, version) { - originResources.Insert(resource) + if !deadOriginVersions.Has(version) { + originResources.Insert(resource) + } } else { - k8sResources.Insert(resource) + if !deadKubernetesVersions.Has(version) { + k8sResources.Insert(resource) + } } } } @@ -195,7 +201,6 @@ func (c *AssetConfig) addHandlers(mux *http.ServeMux) error { config := assets.WebConsoleConfig{ MasterAddr: masterURL.Host, MasterPrefix: OpenShiftAPIPrefix, - MasterLegacyPrefix: LegacyOpenShiftAPIPrefix, MasterResources: originResources.List(), KubernetesAddr: masterURL.Host, KubernetesPrefix: KubernetesAPIPrefix, diff --git a/pkg/cmd/server/origin/handlers.go b/pkg/cmd/server/origin/handlers.go index 22da388f4b97..6124beef8e4e 100644 --- a/pkg/cmd/server/origin/handlers.go +++ b/pkg/cmd/server/origin/handlers.go @@ -29,7 +29,6 @@ func indexAPIPaths(handler http.Handler) http.Handler { // TODO once we have a MuxHelper we will not need to hardcode this list of paths object := kapi.RootPaths{Paths: []string{ "/api", - "/api/v1beta3", "/api/v1", "/controllers", "/healthz", @@ -37,8 +36,6 @@ func indexAPIPaths(handler http.Handler) http.Handler { "/logs/", "/metrics", "/ready", - "/osapi", - "/osapi/v1beta3", "/oapi", "/oapi/v1", "/swaggerapi/", diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index ed2acb9523c2..3897a35b5861 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -274,14 +274,6 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin legacyAPIVersions := []string{} currentAPIVersions := []string{} - if configapi.HasOpenShiftAPILevel(c.Options, OpenShiftAPIV1Beta3) { - if err := c.api_v1beta3(storage).InstallREST(container); err != nil { - glog.Fatalf("Unable to initialize v1beta3 API: %v", err) - } - messages = append(messages, fmt.Sprintf("Started Origin API at %%s%s", OpenShiftAPIPrefixV1Beta3)) - legacyAPIVersions = append(legacyAPIVersions, OpenShiftAPIV1Beta3) - } - if configapi.HasOpenShiftAPILevel(c.Options, OpenShiftAPIV1) { if err := c.api_v1(storage).InstallREST(container); err != nil { glog.Fatalf("Unable to initialize v1 API: %v", err) @@ -307,6 +299,10 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin container.Add(root) } + // The old API prefix must continue to return 200 (with an empty versions + // list) for backwards compatibility, even though we won't service any other + // requests through the route. Take care when considering whether to delete + // this route. initAPIVersionRoute(root, LegacyOpenShiftAPIPrefix, legacyAPIVersions...) initAPIVersionRoute(root, OpenShiftAPIPrefix, currentAPIVersions...) @@ -502,10 +498,6 @@ func (c *MasterConfig) InstallUnprotectedAPI(container *restful.Container) []str // initAPIVersionRoute initializes the osapi endpoint to behave similar to the upstream api endpoint func initAPIVersionRoute(root *restful.WebService, prefix string, versions ...string) { - if len(versions) == 0 { - return - } - versionHandler := apiserver.APIVersionHandler(versions...) root.Route(root.GET(prefix).To(versionHandler). Doc("list supported server API versions"). diff --git a/pkg/cmd/util/clientcmd/errors_test.go b/pkg/cmd/util/clientcmd/errors_test.go new file mode 100644 index 000000000000..da7649106442 --- /dev/null +++ b/pkg/cmd/util/clientcmd/errors_test.go @@ -0,0 +1,21 @@ +package clientcmd + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestIsCertificateAuthorityUnknown(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + defer server.Close() + + req, _ := http.NewRequest("GET", server.URL, nil) + _, err := http.DefaultClient.Do(req) + if err == nil { + t.Fatalf("Expected TLS error") + } + if !IsCertificateAuthorityUnknown(err) { + t.Fatalf("Expected IsCertificateAuthorityUnknown error, error message was %q", err.Error()) + } +} diff --git a/pkg/cmd/util/clientcmd/factory.go b/pkg/cmd/util/clientcmd/factory.go index 242488d0dfd8..a32c433ea413 100644 --- a/pkg/cmd/util/clientcmd/factory.go +++ b/pkg/cmd/util/clientcmd/factory.go @@ -55,6 +55,7 @@ func NewFactory(clientConfig kclientcmd.ClientConfig) *Factory { clients := &clientCache{ clients: make(map[string]*client.Client), + configs: make(map[string]*kclient.Config), loader: clientConfig, } @@ -71,7 +72,9 @@ func NewFactory(clientConfig kclientcmd.ClientConfig) *Factory { } w.Object = func() (meta.RESTMapper, runtime.ObjectTyper) { - if cfg, err := clientConfig.ClientConfig(); err == nil { + // Output using whatever version was negotiated in the client cache. The + // version we decode with may not be the same as what the server requires. + if cfg, err := clients.ClientConfigForVersion(""); err == nil { return kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersion: cfg.Version}, api.Scheme } return mapper, api.Scheme @@ -259,12 +262,16 @@ func expandResourceShortcut(resource string) string { return resource } -// clientCache caches previously loaded clients for reuse, and ensures MatchServerVersion -// is invoked only once +// clientCache caches previously loaded clients for reuse. This is largely +// copied from upstream (because of typing) but reuses the negotiation logic. +// TODO: Consolidate this entire concept with upstream's ClientCache. type clientCache struct { loader kclientcmd.ClientConfig clients map[string]*client.Client + configs map[string]*kclient.Config defaultConfig *kclient.Config + // negotiatingClient is used only for negotiating versions with the server. + negotiatingClient *kclient.Client } // ClientConfigForVersion returns the correct config for a server @@ -277,11 +284,36 @@ func (c *clientCache) ClientConfigForVersion(version string) (*kclient.Config, e c.defaultConfig = config } // TODO: have a better config copy method + if config, ok := c.configs[version]; ok { + return config, nil + } + if c.negotiatingClient == nil { + // TODO: We want to reuse the upstream negotiation logic, which is coupled + // to a concrete kube Client. The negotiation will ultimately try and + // build an unversioned URL using the config prefix to ask for supported + // server versions. If we use the default kube client config, the prefix + // will be /api, while we need to use the OpenShift prefix to ask for the + // OpenShift server versions. For now, set OpenShift defaults on the + // config to ensure the right prefix gets used. The client cache and + // negotiation logic should be refactored upstream to support downstream + // reuse so that we don't need to do any of this cache or negotiation + // duplication. + negotiatingConfig := *c.defaultConfig + client.SetOpenShiftDefaults(&negotiatingConfig) + negotiatingClient, err := kclient.New(&negotiatingConfig) + if err != nil { + return nil, err + } + c.negotiatingClient = negotiatingClient + } config := *c.defaultConfig - if len(version) != 0 { - config.Version = version + negotiatedVersion, err := kclient.NegotiateVersion(c.negotiatingClient, &config, version, latest.Versions) + if err != nil { + return nil, err } + config.Version = negotiatedVersion client.SetOpenShiftDefaults(&config) + c.configs[version] = &config return &config, nil } @@ -289,15 +321,13 @@ func (c *clientCache) ClientConfigForVersion(version string) (*kclient.Config, e // ClientForVersion initializes or reuses a client for the specified version, or returns an // error if that is not possible func (c *clientCache) ClientForVersion(version string) (*client.Client, error) { + if client, ok := c.clients[version]; ok { + return client, nil + } config, err := c.ClientConfigForVersion(version) if err != nil { return nil, err } - - if client, ok := c.clients[config.Version]; ok { - return client, nil - } - client, err := client.New(config) if err != nil { return nil, err diff --git a/pkg/cmd/util/clientcmd/factory_test.go b/pkg/cmd/util/clientcmd/factory_test.go new file mode 100644 index 000000000000..3d8c52d2a199 --- /dev/null +++ b/pkg/cmd/util/clientcmd/factory_test.go @@ -0,0 +1,71 @@ +package clientcmd + +import ( + "net/http" + "net/http/httptest" + "testing" + + kclient "k8s.io/kubernetes/pkg/client/unversioned" + + "github.com/openshift/origin/pkg/client" +) + +func TestClientConfigForVersion(t *testing.T) { + called := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path != "/oapi" { + t.Fatalf("Unexpected path called during negotiation: %s", req.URL.Path) + } + called++ + w.Write([]byte(`{"versions":["v1"]}`)) + })) + defer server.Close() + + defaultConfig := &kclient.Config{Host: server.URL} + client.SetOpenShiftDefaults(defaultConfig) + + clients := &clientCache{ + clients: make(map[string]*client.Client), + configs: make(map[string]*kclient.Config), + defaultConfig: defaultConfig, + } + + // First call, negotiate + called = 0 + v1Config, err := clients.ClientConfigForVersion("v1") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if v1Config.Version != "v1" { + t.Fatalf("Expected v1, got %v", v1Config.Version) + } + if called != 1 { + t.Fatalf("Expected to be called 1 time during negotiation, was called %d times", called) + } + + // Second call, cache + called = 0 + v1Config, err = clients.ClientConfigForVersion("v1") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if v1Config.Version != "v1" { + t.Fatalf("Expected v1, got %v", v1Config.Version) + } + if called != 0 { + t.Fatalf("Expected not be called again getting a config from cache, was called %d additional times", called) + } + + // Call for unsupported version, negotiate to supported version + called = 0 + v1beta3Config, err := clients.ClientConfigForVersion("v1beta3") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if v1beta3Config.Version != "v1" { + t.Fatalf("Expected to negotiate v1 for v1beta3 config, got %v", v1beta3Config.Version) + } + if called != 1 { + t.Fatalf("Expected to be called once getting a config for a new version, was called %d times", called) + } +} diff --git a/test/cmd/basicresources.sh b/test/cmd/basicresources.sh index ae4a5b62da06..da72630d3f8c 100755 --- a/test/cmd/basicresources.sh +++ b/test/cmd/basicresources.sh @@ -43,6 +43,11 @@ oc create -f test/integration/fixtures/test-service.json oc delete services frontend echo "services: ok" +oc create -f test/fixtures/mixed-api-versions.yaml +oc get -f test/fixtures/mixed-api-versions.yaml -o yaml > /dev/null +oc delete -f test/fixtures/mixed-api-versions.yaml +echo "list version conversion: ok" + oc get nodes ( # subshell so we can unset kubeconfig diff --git a/test/cmd/builds.sh b/test/cmd/builds.sh index d241a419f063..3f121f3ae0bc 100755 --- a/test/cmd/builds.sh +++ b/test/cmd/builds.sh @@ -32,15 +32,13 @@ oc get bc/ruby-sample-build -t '{{ .spec.output.to.name }}' | grep 'different' oc patch bc/ruby-sample-build -p "{\"spec\":{\"output\":{\"to\":{\"name\":\"${REAL_OUTPUT_TO}\"}}}}" echo "patchAnonFields: ok" -[ "$(oc describe buildConfigs ruby-sample-build --api-version=v1beta3 | grep --text "Webhook GitHub" | grep -F "${url}/osapi/v1beta3/namespaces/${project}/buildconfigs/ruby-sample-build/webhooks/secret101/github")" ] -[ "$(oc describe buildConfigs ruby-sample-build --api-version=v1beta3 | grep --text "Webhook Generic" | grep -F "${url}/osapi/v1beta3/namespaces/${project}/buildconfigs/ruby-sample-build/webhooks/secret101/generic")" ] +[ "$(oc describe buildConfigs ruby-sample-build | grep --text "Webhook GitHub" | grep -F "${url}/oapi/v1/namespaces/${project}/buildconfigs/ruby-sample-build/webhooks/secret101/github")" ] +[ "$(oc describe buildConfigs ruby-sample-build | grep --text "Webhook Generic" | grep -F "${url}/oapi/v1/namespaces/${project}/buildconfigs/ruby-sample-build/webhooks/secret101/generic")" ] oc start-build --list-webhooks='all' ruby-sample-build [ "$(oc start-build --list-webhooks='all' ruby-sample-build | grep --text "generic")" ] [ "$(oc start-build --list-webhooks='all' ruby-sample-build | grep --text "github")" ] [ "$(oc start-build --list-webhooks='github' ruby-sample-build | grep --text "secret101")" ] [ ! "$(oc start-build --list-webhooks='blah')" ] -webhook=$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1beta3 | head -n 1) -oc start-build --from-webhook="${webhook}" webhook=$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1) oc start-build --from-webhook="${webhook}" oc get builds diff --git a/test/fixtures/mixed-api-versions.yaml b/test/fixtures/mixed-api-versions.yaml new file mode 100644 index 000000000000..37ed469e76c0 --- /dev/null +++ b/test/fixtures/mixed-api-versions.yaml @@ -0,0 +1,41 @@ +apiVersion: v1 +kind: List +metadata: {} +items: + +- apiVersion: v1 + kind: Secret + metadata: + annotations: + description: v1 Secret - used to test v1 negotiation of k8s objects + name: v1-secret + +- apiVersion: v1beta3 + kind: Secret + metadata: + annotations: + description: v1beta3 Secret - used to test v1beta3 negotiation of k8s objects + name: v1beta3-secret + +- apiVersion: v1 + kind: Route + metadata: + annotations: + description: v1 Route - used to test v1 negotiation of origin objects + name: v1-route + spec: + to: + kind: Service + name: test + +- apiVersion: v1beta3 + kind: Route + metadata: + annotations: + description: v1beta3 Route - used to test v1beta3 negotiation of origin objects + name: v1beta3-route + spec: + to: + kind: Service + name: test + diff --git a/test/integration/router/router_http_server.go b/test/integration/router/router_http_server.go index 40e887032f69..b797ca2960d7 100644 --- a/test/integration/router/router_http_server.go +++ b/test/integration/router/router_http_server.go @@ -181,8 +181,6 @@ func (s *TestHttpService) startMaster() error { for _, version := range apis { masterServer.HandleFunc(fmt.Sprintf("/api/%s/endpoints", version), s.handleEndpointList) masterServer.HandleFunc(fmt.Sprintf("/api/%s/watch/endpoints", version), s.handleEndpointWatch) - masterServer.HandleFunc(fmt.Sprintf("/osapi/%s/routes", version), s.handleRouteList) - masterServer.HandleFunc(fmt.Sprintf("/osapi/%s/watch/routes", version), s.handleRouteWatch) masterServer.HandleFunc(fmt.Sprintf("/oapi/%s/routes", version), s.handleRouteList) masterServer.HandleFunc(fmt.Sprintf("/oapi/%s/watch/routes", version), s.handleRouteWatch) } diff --git a/test/integration/template_test.go b/test/integration/template_test.go index 80cf4d00154d..710f11fc49e6 100644 --- a/test/integration/template_test.go +++ b/test/integration/template_test.go @@ -5,7 +5,7 @@ package integration import ( "testing" - "k8s.io/kubernetes/pkg/api/v1beta3" + "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/runtime" "github.com/openshift/origin/pkg/client" @@ -22,7 +22,7 @@ func TestTemplate(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - for _, version := range []string{"v1", "v1beta3"} { + for _, version := range []string{"v1"} { config, err := testutil.GetClusterAdminClientConfig(path) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -42,13 +42,13 @@ func TestTemplate(t *testing.T) { }, }, Objects: []runtime.Object{ - &v1beta3.Service{ - ObjectMeta: v1beta3.ObjectMeta{ + &v1.Service{ + ObjectMeta: v1.ObjectMeta{ Name: "${NAME}-tester", Namespace: "somevalue", }, - Spec: v1beta3.ServiceSpec{ - PortalIP: "1.2.3.4", + Spec: v1.ServiceSpec{ + ClusterIP: "1.2.3.4", SessionAffinity: "some-bad-${VALUE}", }, }, @@ -69,7 +69,7 @@ func TestTemplate(t *testing.T) { spec := svc["spec"].(map[string]interface{}) meta := svc["metadata"].(map[string]interface{}) // keep existing values - if spec["portalIP"] != "1.2.3.4" { + if spec["clusterIP"] != "1.2.3.4" { t.Fatalf("unexpected object: %#v", svc) } // replace a value diff --git a/test/old-start-configs/v1.0.0/test-end-to-end.sh b/test/old-start-configs/v1.0.0/test-end-to-end.sh index 322e9a9781b6..742286088722 100755 --- a/test/old-start-configs/v1.0.0/test-end-to-end.sh +++ b/test/old-start-configs/v1.0.0/test-end-to-end.sh @@ -275,7 +275,7 @@ fi wait_for_url "${KUBELET_SCHEME}://${KUBELET_HOST}:${KUBELET_PORT}/healthz" "[INFO] kubelet: " 0.5 60 wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "apiserver: " 0.25 80 wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz/ready" "apiserver(ready): " 0.25 80 -wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1beta3/nodes/${KUBELET_HOST}" "apiserver(nodes): " 0.25 80 +wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1/nodes/${KUBELET_HOST}" "apiserver(nodes): " 0.25 80 # COMPATIBILITY update the cluster roles and role bindings so that new images can be used. oadm policy reconcile-cluster-roles --confirm