From 87f8903c868a41f8699afd7444a204fcee0f7caa Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Fri, 25 Aug 2023 11:37:58 +0200 Subject: [PATCH 01/14] init --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 8abbdd11a3..6050d0f9e2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -69,7 +69,7 @@ type Config struct { Name string `yaml:"name" validate:"valid_name,required"` // description: | // Kubernetes version to be installed into the cluster. - KubernetesVersion string `yaml:"kubernetesVersion" validate:"required,supported_k8s_version"` + KubernetesVersion versions.ValidK8sVersion `yaml:"kubernetesVersion" validate:"required,supported_k8s_version"` // description: | // Microservice version to be installed into the cluster. Defaults to the version of the CLI. MicroserviceVersion semver.Semver `yaml:"microserviceVersion" validate:"required"` From 440cae231667ec9a92f66522ebb7dc3eea26800d Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Fri, 25 Aug 2023 16:11:36 +0200 Subject: [PATCH 02/14] init --- cli/internal/cmd/configgenerate.go | 42 ++---------- cli/internal/cmd/configgenerate_test.go | 31 +++++++-- cli/internal/cmd/init.go | 10 +-- cli/internal/cmd/init_test.go | 10 ++- cli/internal/cmd/upgradeapply.go | 31 ++------- cli/internal/cmd/upgradecheck.go | 2 +- cli/internal/helm/BUILD.bazel | 1 - cli/internal/helm/helm.go | 15 ++--- cli/internal/helm/helm_test.go | 3 +- cli/internal/helm/loader.go | 3 +- cli/internal/helm/loader_test.go | 4 +- cli/internal/kubecmd/kubecmd.go | 10 +-- cli/internal/kubecmd/kubecmd_test.go | 50 +++++++------- docs/docs/reference/cli.md | 2 +- e2e/internal/upgrade/upgrade_test.go | 21 +++--- internal/config/BUILD.bazel | 1 - internal/config/config.go | 2 +- internal/config/config_doc.go | 2 +- internal/config/config_test.go | 35 ++++------ internal/config/migration/BUILD.bazel | 1 + internal/config/migration/migration.go | 3 +- internal/config/validation.go | 16 ----- internal/versions/versions.go | 88 +++++++++++++++++++++++-- 23 files changed, 202 insertions(+), 181 deletions(-) diff --git a/cli/internal/cmd/configgenerate.go b/cli/internal/cmd/configgenerate.go index 6e05a6b920..8a582dfe18 100644 --- a/cli/internal/cmd/configgenerate.go +++ b/cli/internal/cmd/configgenerate.go @@ -13,14 +13,13 @@ import ( "github.com/edgelesssys/constellation/v2/cli/internal/cmd/pathprefix" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" - "github.com/edgelesssys/constellation/v2/internal/compatibility" "github.com/edgelesssys/constellation/v2/internal/config" + "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/spf13/afero" "github.com/spf13/cobra" - "golang.org/x/mod/semver" ) func newConfigGenerateCmd() *cobra.Command { @@ -35,7 +34,7 @@ func newConfigGenerateCmd() *cobra.Command { ValidArgsFunction: generateCompletion, RunE: runConfigGenerate, } - cmd.Flags().StringP("kubernetes", "k", semver.MajorMinor(config.Default().KubernetesVersion), "Kubernetes version to use in format MAJOR.MINOR") + cmd.Flags().StringP("kubernetes", "k", string(config.Default().KubernetesVersion), "Kubernetes version to use in format MAJOR.MINOR") cmd.Flags().StringP("attestation", "a", "", fmt.Sprintf("attestation variant to use %s. If not specified, the default for the cloud provider is used", printFormattedSlice(variant.GetAvailableAttestationVariants()))) return cmd @@ -43,7 +42,7 @@ func newConfigGenerateCmd() *cobra.Command { type generateFlags struct { pf pathprefix.PathPrefixer - k8sVersion string + k8sVersion versions.ValidK8sVersion attestationVariant variant.Variant } @@ -124,19 +123,6 @@ func createConfig(provider cloudprovider.Provider) *config.Config { return res } -// supportedVersions prints the supported version without v prefix and without patch version. -// Should only be used when accepting Kubernetes versions from --kubernetes. -func supportedVersions() string { - builder := strings.Builder{} - for i, version := range versions.SupportedK8sVersions() { - if i > 0 { - builder.WriteString(" ") - } - builder.WriteString(strings.TrimPrefix(semver.MajorMinor(version), "v")) - } - return builder.String() -} - func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) { workDir, err := cmd.Flags().GetString("workspace") if err != nil { @@ -144,11 +130,11 @@ func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) { } k8sVersion, err := cmd.Flags().GetString("kubernetes") if err != nil { - return generateFlags{}, fmt.Errorf("parsing kuberentes flag: %w", err) + return generateFlags{}, fmt.Errorf("parsing Kubernetes flag: %w", err) } - resolvedVersion, err := resolveK8sVersion(k8sVersion) + resolvedVersion, err := versions.NewValidK8sVersion(k8sVersion, true) // allow versions without specified patch if err != nil { - return generateFlags{}, fmt.Errorf("resolving kuberentes version from flag: %w", err) + return generateFlags{}, fmt.Errorf("resolving Kubernetes version from flag: %w", err) } attestationString, err := cmd.Flags().GetString("attestation") @@ -184,22 +170,6 @@ func generateCompletion(_ *cobra.Command, args []string, _ string) ([]string, co } } -// resolveK8sVersion takes the user input from --kubernetes and transforms a MAJOR.MINOR definition into a supported -// MAJOR.MINOR.PATCH release. -func resolveK8sVersion(k8sVersion string) (string, error) { - prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) - if !semver.IsValid(prefixedVersion) { - return "", fmt.Errorf("kubernetes flag does not specify a valid semantic version: %s", k8sVersion) - } - - extendedVersion := config.K8sVersionFromMajorMinor(prefixedVersion) - if extendedVersion == "" { - return "", fmt.Errorf("--kubernetes (%s) does not specify a valid Kubernetes version. Supported versions: %s", strings.TrimPrefix(k8sVersion, "v"), supportedVersions()) - } - - return extendedVersion, nil -} - func printFormattedSlice[T any](input []T) string { return fmt.Sprintf("{%s}", strings.Join(toString(input), "|")) } diff --git a/cli/internal/cmd/configgenerate_test.go b/cli/internal/cmd/configgenerate_test.go index d11cd5d806..9a07c7ad04 100644 --- a/cli/internal/cmd/configgenerate_test.go +++ b/cli/internal/cmd/configgenerate_test.go @@ -8,6 +8,7 @@ package cmd import ( "fmt" + "strings" "testing" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" @@ -29,9 +30,29 @@ func TestConfigGenerateKubernetesVersion(t *testing.T) { version string wantErr bool }{ - "success": { + "default version": { + version: "", + }, + "without v prefix": { + version: strings.TrimPrefix(string(versions.Default), "v"), + }, + "K8s version without patch version": { version: semver.MajorMinor(string(versions.Default)), }, + "K8s version with patch version": { + version: string(versions.Default), + }, + "K8s version with invalid patch version": { + version: func() string { + s := string(versions.Default) + return s[:len(s)-1] + "99" + }(), + wantErr: true, + }, + "outdated K8s version": { + version: "v1.0.0", + wantErr: true, + }, "no semver": { version: "asdf", wantErr: true, @@ -50,11 +71,13 @@ func TestConfigGenerateKubernetesVersion(t *testing.T) { fileHandler := file.NewHandler(afero.NewMemMapFs()) cmd := newConfigGenerateCmd() cmd.Flags().String("workspace", "", "") // register persistent flag manually - err := cmd.Flags().Set("kubernetes", tc.version) - require.NoError(err) + if tc.version != "" { + err := cmd.Flags().Set("kubernetes", tc.version) + require.NoError(err) + } cg := &configGenerateCmd{log: logger.NewTest(t)} - err = cg.configGenerate(cmd, fileHandler, cloudprovider.Unknown, "") + err := cg.configGenerate(cmd, fileHandler, cloudprovider.Unknown, "") if tc.wantErr { assert.Error(err) diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index 1781b13a5b..c3f90b6785 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -24,7 +24,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/api/attestationconfigapi" "github.com/edgelesssys/constellation/v2/internal/atls" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" - "github.com/edgelesssys/constellation/v2/internal/compatibility" "github.com/spf13/afero" "github.com/spf13/cobra" @@ -170,10 +169,7 @@ func (i *initCmd) initialize( // config validation does not check k8s patch version since upgrade may accept an outdated patch version. // init only supported up-to-date versions. - k8sVersion, err := versions.NewValidK8sVersion(compatibility.EnsurePrefixV(conf.KubernetesVersion), true) - if err != nil { - return err - } + k8sVersion := conf.KubernetesVersion i.log.Debugf("Validated k8s version as %s", k8sVersion) if versions.IsPreviewK8sVersion(k8sVersion) { cmd.PrintErrf("Warning: Constellation with Kubernetes %v is still in preview. Use only for evaluation purposes.\n", k8sVersion) @@ -275,7 +271,7 @@ func (i *initCmd) initialize( if err != nil { return fmt.Errorf("creating Helm client: %w", err) } - executor, includesUpgrades, err := helmApplier.PrepareApply(conf, k8sVersion, idFile, options, output, + executor, includesUpgrades, err := helmApplier.PrepareApply(conf, idFile, options, output, serviceAccURI, masterSecret) if err != nil { return fmt.Errorf("getting Helm chart executor: %w", err) @@ -629,7 +625,7 @@ type attestationConfigApplier interface { } type helmApplier interface { - PrepareApply(conf *config.Config, validK8sversion versions.ValidK8sVersion, idFile clusterid.File, flags helm.Options, tfOutput terraform.ApplyOutput, serviceAccURI string, masterSecret uri.MasterSecret) (helm.Applier, bool, error) + PrepareApply(conf *config.Config, idFile clusterid.File, flags helm.Options, tfOutput terraform.ApplyOutput, serviceAccURI string, masterSecret uri.MasterSecret) (helm.Applier, bool, error) } type clusterShower interface { diff --git a/cli/internal/cmd/init_test.go b/cli/internal/cmd/init_test.go index 13ae7eb4a6..9d34438fc5 100644 --- a/cli/internal/cmd/init_test.go +++ b/cli/internal/cmd/init_test.go @@ -133,7 +133,13 @@ func TestInitialize(t *testing.T) { provider: cloudprovider.Azure, idFile: &clusterid.File{IP: "192.0.2.1"}, initServerAPI: &stubInitServer{res: &initproto.InitResponse{Kind: &initproto.InitResponse_InitSuccess{InitSuccess: testInitResp}}}, - configMutator: func(c *config.Config) { c.KubernetesVersion = strings.TrimPrefix(string(versions.Default), "v") }, + configMutator: func(c *config.Config) { + res, err := versions.NewValidK8sVersion(strings.TrimPrefix(string(versions.Default), "v"), true) + if err != nil { + panic("invalid k8s version") + } + c.KubernetesVersion = res + }, }, } @@ -222,7 +228,7 @@ type stubApplier struct { err error } -func (s stubApplier) PrepareApply(_ *config.Config, _ versions.ValidK8sVersion, _ clusterid.File, _ helm.Options, _ terraform.ApplyOutput, _ string, _ uri.MasterSecret) (helm.Applier, bool, error) { +func (s stubApplier) PrepareApply(_ *config.Config, _ clusterid.File, _ helm.Options, _ terraform.ApplyOutput, _ string, _ uri.MasterSecret) (helm.Applier, bool, error) { return stubRunner{}, false, s.err } diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 6fa6e6fdc3..e581b7432d 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -166,7 +166,9 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl } } } - validK8sVersion, err := validK8sVersion(cmd, conf.KubernetesVersion, flags.yes) + if versions.IsPreviewK8sVersion(conf.KubernetesVersion) { + cmd.PrintErrf("Warning: Constellation with Kubernetes %q is still in preview. Use only for evaluation purposes.\n", conf.KubernetesVersion) + } if err != nil { return err } @@ -223,7 +225,7 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl var upgradeErr *compatibility.InvalidUpgradeError if !flags.skipPhases.contains(skipHelmPhase) { - err = u.handleServiceUpgrade(cmd, conf, idFile, tfOutput, validK8sVersion, upgradeDir, flags) + err = u.handleServiceUpgrade(cmd, conf, idFile, tfOutput, upgradeDir, flags) switch { case errors.As(err, &upgradeErr): cmd.PrintErrln(err) @@ -335,27 +337,6 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, conf *config.Conf return tfOutput, nil } -// validK8sVersion checks if the Kubernetes patch version is supported and asks for confirmation if not. -func validK8sVersion(cmd *cobra.Command, version string, yes bool) (validVersion versions.ValidK8sVersion, err error) { - validVersion, err = versions.NewValidK8sVersion(version, true) - if versions.IsPreviewK8sVersion(validVersion) { - cmd.PrintErrf("Warning: Constellation with Kubernetes %v is still in preview. Use only for evaluation purposes.\n", validVersion) - } - valid := err == nil - - if !valid && !yes { - confirmed, err := askToConfirm(cmd, fmt.Sprintf("WARNING: The Kubernetes patch version %s is not supported. If you continue, Kubernetes upgrades will be skipped. Do you want to continue anyway?", version)) - if err != nil { - return validVersion, fmt.Errorf("asking for confirmation: %w", err) - } - if !confirmed { - return validVersion, fmt.Errorf("aborted by user") - } - } - - return validVersion, nil -} - // confirmAndUpgradeAttestationConfig checks if the locally configured measurements are different from the cluster's measurements. // If so the function will ask the user to confirm (if --yes is not set) and upgrade the cluster's config. func (u *upgradeApplyCmd) confirmAndUpgradeAttestationConfig( @@ -400,7 +381,7 @@ func (u *upgradeApplyCmd) confirmAndUpgradeAttestationConfig( func (u *upgradeApplyCmd) handleServiceUpgrade( cmd *cobra.Command, conf *config.Config, idFile clusterid.File, tfOutput terraform.ApplyOutput, - validK8sVersion versions.ValidK8sVersion, upgradeDir string, flags upgradeApplyFlags, + upgradeDir string, flags upgradeApplyFlags, ) error { var secret uri.MasterSecret if err := u.fileHandler.ReadJSON(constants.MasterSecretFilename, &secret); err != nil { @@ -418,7 +399,7 @@ func (u *upgradeApplyCmd) handleServiceUpgrade( prepareApply := func(allowDestructive bool) (helm.Applier, bool, error) { options.AllowDestructive = allowDestructive - executor, includesUpgrades, err := u.helmApplier.PrepareApply(conf, validK8sVersion, idFile, options, + executor, includesUpgrades, err := u.helmApplier.PrepareApply(conf, idFile, options, tfOutput, serviceAccURI, secret) var upgradeErr *compatibility.InvalidUpgradeError switch { diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index d2b05eda6c..30e39a4bc8 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -576,7 +576,7 @@ func (v *versionUpgrade) writeConfig(conf *config.Config, fileHandler file.Handl conf.MicroserviceVersion = v.newServices } if len(v.newKubernetes) > 0 { - conf.KubernetesVersion = v.newKubernetes[0] + conf.KubernetesVersion = versions.ValidK8sVersion(v.newKubernetes[0]) } if len(v.newImages) > 0 { imageUpgrade := sortedMapKeys(v.newImages)[0] diff --git a/cli/internal/helm/BUILD.bazel b/cli/internal/helm/BUILD.bazel index 29a19ea4b8..be59cb0e94 100644 --- a/cli/internal/helm/BUILD.bazel +++ b/cli/internal/helm/BUILD.bazel @@ -473,7 +473,6 @@ go_test( "//internal/kms/uri", "//internal/logger", "//internal/semver", - "//internal/versions", "@com_github_pkg_errors//:errors", "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//mock", diff --git a/cli/internal/helm/helm.go b/cli/internal/helm/helm.go index 5c285cc87c..060253851b 100644 --- a/cli/internal/helm/helm.go +++ b/cli/internal/helm/helm.go @@ -40,7 +40,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/kms/uri" "github.com/edgelesssys/constellation/v2/internal/kubernetes/kubectl" "github.com/edgelesssys/constellation/v2/internal/semver" - "github.com/edgelesssys/constellation/v2/internal/versions" ) const ( @@ -87,11 +86,10 @@ type Options struct { // PrepareApply loads the charts and returns the executor to apply them. // TODO(elchead): remove validK8sVersion by putting ValidK8sVersion into config.Config, see AB#3374. -func (h Client) PrepareApply( - conf *config.Config, validK8sversion versions.ValidK8sVersion, idFile clusterid.File, - flags Options, tfOutput terraform.ApplyOutput, serviceAccURI string, masterSecret uri.MasterSecret, +func (h Client) PrepareApply(conf *config.Config, idFile clusterid.File, flags Options, tfOutput terraform.ApplyOutput, + serviceAccURI string, masterSecret uri.MasterSecret, ) (Applier, bool, error) { - releases, err := h.loadReleases(conf, masterSecret, validK8sversion, idFile, flags, tfOutput, serviceAccURI) + releases, err := h.loadReleases(conf, masterSecret, idFile, flags, tfOutput, serviceAccURI) if err != nil { return nil, false, fmt.Errorf("loading Helm releases: %w", err) } @@ -100,11 +98,10 @@ func (h Client) PrepareApply( return &ChartApplyExecutor{actions: actions, log: h.log}, includesUpgrades, err } -func (h Client) loadReleases( - conf *config.Config, secret uri.MasterSecret, validK8sVersion versions.ValidK8sVersion, - idFile clusterid.File, flags Options, tfOutput terraform.ApplyOutput, serviceAccURI string, +func (h Client) loadReleases(conf *config.Config, secret uri.MasterSecret, idFile clusterid.File, flags Options, + tfOutput terraform.ApplyOutput, serviceAccURI string, ) ([]Release, error) { - helmLoader := newLoader(conf, idFile, validK8sVersion, h.cliVersion) + helmLoader := newLoader(conf, idFile, h.cliVersion) h.log.Debugf("Created new Helm loader") return helmLoader.loadReleases(flags.Conformance, flags.HelmWaitMode, secret, serviceAccURI, tfOutput) diff --git a/cli/internal/helm/helm_test.go b/cli/internal/helm/helm_test.go index c39551e8ec..b160c35cf9 100644 --- a/cli/internal/helm/helm_test.go +++ b/cli/internal/helm/helm_test.go @@ -18,7 +18,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/kms/uri" "github.com/edgelesssys/constellation/v2/internal/logger" "github.com/edgelesssys/constellation/v2/internal/semver" - "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "helm.sh/helm/v3/pkg/action" @@ -208,7 +207,7 @@ func TestHelmApply(t *testing.T) { helmListVersion(lister, "aws-load-balancer-controller", awsLbVersion) options.AllowDestructive = tc.allowDestructive - ex, includesUpgrade, err := sut.PrepareApply(cfg, versions.ValidK8sVersion("v1.27.4"), + ex, includesUpgrade, err := sut.PrepareApply(cfg, clusterid.File{UID: "testuid", MeasurementSalt: []byte("measurementSalt")}, options, fakeTerraformOutput(csp), fakeServiceAccURI(csp), uri.MasterSecret{Key: []byte("secret"), Salt: []byte("masterSalt")}) diff --git a/cli/internal/helm/loader.go b/cli/internal/helm/loader.go index acdbc6c302..8992dd670b 100644 --- a/cli/internal/helm/loader.go +++ b/cli/internal/helm/loader.go @@ -77,11 +77,12 @@ type chartLoader struct { } // newLoader creates a new ChartLoader. -func newLoader(config *config.Config, idFile clusterid.File, k8sVersion versions.ValidK8sVersion, cliVersion semver.Semver) *chartLoader { +func newLoader(config *config.Config, idFile clusterid.File, cliVersion semver.Semver) *chartLoader { // TODO(malt3): Allow overriding container image registry + prefix for all images // (e.g. for air-gapped environments). var ccmImage, cnmImage string csp := config.GetProvider() + k8sVersion := config.KubernetesVersion switch csp { case cloudprovider.AWS: ccmImage = versions.VersionConfigs[k8sVersion].CloudControllerManagerImageAWS diff --git a/cli/internal/helm/loader_test.go b/cli/internal/helm/loader_test.go index 1eb385662d..ba1122ea28 100644 --- a/cli/internal/helm/loader_test.go +++ b/cli/internal/helm/loader_test.go @@ -31,7 +31,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/config" "github.com/edgelesssys/constellation/v2/internal/kms/uri" "github.com/edgelesssys/constellation/v2/internal/semver" - "github.com/edgelesssys/constellation/v2/internal/versions" ) func fakeServiceAccURI(provider cloudprovider.Provider) string { @@ -67,9 +66,8 @@ func TestLoadReleases(t *testing.T) { assert := assert.New(t) require := require.New(t) config := &config.Config{Provider: config.ProviderConfig{GCP: &config.GCPConfig{}}} - k8sVersion := versions.ValidK8sVersion("v1.27.4") chartLoader := newLoader(config, clusterid.File{UID: "testuid", MeasurementSalt: []byte("measurementSalt")}, - k8sVersion, semver.NewFromInt(2, 10, 0, "")) + semver.NewFromInt(2, 10, 0, "")) helmReleases, err := chartLoader.loadReleases( true, WaitModeAtomic, uri.MasterSecret{Key: []byte("secret"), Salt: []byte("masterSalt")}, diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index fee16e32e0..4f9f84eded 100644 --- a/cli/internal/kubecmd/kubecmd.go +++ b/cli/internal/kubecmd/kubecmd.go @@ -138,12 +138,14 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f // We have to allow users to specify outdated k8s patch versions. // Therefore, this code has to skip k8s updates if a user configures an outdated (i.e. invalid) k8s version. var components *corev1.ConfigMap - currentK8sVersion, err := versions.NewValidK8sVersion(conf.KubernetesVersion, true) + _, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) if err != nil { - innerErr := fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(versions.SupportedK8sVersions(), ", ")) - err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, conf.KubernetesVersion, innerErr) + innerErr := fmt.Errorf("unsupported Kubernetes version, supported versions are %s", + strings.Join(versions.SupportedK8sVersions(), ", ")) + err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, + string(conf.KubernetesVersion), innerErr) } else { - versionConfig := versions.VersionConfigs[currentK8sVersion] + versionConfig := versions.VersionConfigs[conf.KubernetesVersion] components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) } diff --git a/cli/internal/kubecmd/kubecmd_test.go b/cli/internal/kubecmd/kubecmd_test.go index ec24a274bd..f377493b05 100644 --- a/cli/internal/kubecmd/kubecmd_test.go +++ b/cli/internal/kubecmd/kubecmd_test.go @@ -43,7 +43,7 @@ func TestUpgradeNodeVersion(t *testing.T) { currentImageVersion string newImageReference string badImageVersion string - currentClusterVersion string + currentClusterVersion versions.ValidK8sVersion conf *config.Config force bool getCRErr error @@ -56,11 +56,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -72,11 +72,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -93,11 +93,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[0] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[0] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -114,11 +114,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[0] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[0] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{}, wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { @@ -130,7 +130,7 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), conditions: []metav1.Condition{{ @@ -138,7 +138,7 @@ func TestUpgradeNodeVersion(t *testing.T) { Status: metav1.ConditionTrue, }}, currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{}, wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { @@ -149,7 +149,7 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), conditions: []metav1.Condition{{ @@ -157,7 +157,7 @@ func TestUpgradeNodeVersion(t *testing.T) { Status: metav1.ConditionTrue, }}, currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{}, force: true, wantUpdate: true, @@ -166,11 +166,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -186,12 +186,12 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.4.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), newImageReference: "path/to/image:v1.4.2", currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":true}}`), @@ -208,12 +208,12 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.4.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), newImageReference: "path/to/image:v1.4.2", currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -226,11 +226,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], badImageVersion: "v3.2.1", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ @@ -252,7 +252,7 @@ func TestUpgradeNodeVersion(t *testing.T) { return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -269,11 +269,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentClusterVersion: versions.SupportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -298,7 +298,7 @@ func TestUpgradeNodeVersion(t *testing.T) { nodeVersion := updatev1alpha1.NodeVersion{ Spec: updatev1alpha1.NodeVersionSpec{ ImageVersion: tc.currentImageVersion, - KubernetesClusterVersion: tc.currentClusterVersion, + KubernetesClusterVersion: string(tc.currentClusterVersion), }, Status: updatev1alpha1.NodeVersionStatus{ Conditions: tc.conditions, diff --git a/docs/docs/reference/cli.md b/docs/docs/reference/cli.md index 2d9a8ebff3..09991cb633 100644 --- a/docs/docs/reference/cli.md +++ b/docs/docs/reference/cli.md @@ -79,7 +79,7 @@ constellation config generate {aws|azure|gcp|openstack|qemu|stackit} [flags] ``` -a, --attestation string attestation variant to use {aws-sev-snp|aws-nitro-tpm|azure-sev-snp|azure-trustedlaunch|gcp-sev-es|qemu-vtpm}. If not specified, the default for the cloud provider is used -h, --help help for generate - -k, --kubernetes string Kubernetes version to use in format MAJOR.MINOR (default "v1.27") + -k, --kubernetes string Kubernetes version to use in format MAJOR.MINOR (default "v1.27.4") ``` ### Options inherited from parent commands diff --git a/e2e/internal/upgrade/upgrade_test.go b/e2e/internal/upgrade/upgrade_test.go index 1c5521767b..c33346bc54 100644 --- a/e2e/internal/upgrade/upgrade_test.go +++ b/e2e/internal/upgrade/upgrade_test.go @@ -273,12 +273,11 @@ func writeUpgradeConfig(require *require.Assertions, image string, kubernetes st cfg.Image = image defaultConfig := config.Default() - var kubernetesVersion semver.Semver + var kubernetesVersion versions.ValidK8sVersion if kubernetes == "" { - kubernetesVersion, err = semver.New(defaultConfig.KubernetesVersion) - require.NoError(err) + kubernetesVersion = defaultConfig.KubernetesVersion } else { - kubernetesVersion, err = semver.New(kubernetes) + kubernetesVersion, err = versions.NewValidK8sVersion(kubernetes, true) require.NoError(err) } @@ -291,8 +290,8 @@ func writeUpgradeConfig(require *require.Assertions, image string, kubernetes st microserviceVersion = version } - log.Printf("Setting K8s version: %s\n", kubernetesVersion.String()) - cfg.KubernetesVersion = kubernetesVersion.String() + log.Printf("Setting K8s version: %s\n", kubernetesVersion) + cfg.KubernetesVersion = kubernetesVersion log.Printf("Setting microservice version: %s\n", microserviceVersion) cfg.MicroserviceVersion = microserviceVersion @@ -432,13 +431,13 @@ func testNodesEventuallyHaveVersion(t *testing.T, k *kubernetes.Clientset, targe } kubeletVersion := node.Status.NodeInfo.KubeletVersion - if kubeletVersion != targetVersions.kubernetes.String() { - log.Printf("\t%s: K8s (Kubelet) %s, want %s\n", node.Name, kubeletVersion, targetVersions.kubernetes.String()) + if kubeletVersion != string(targetVersions.kubernetes) { + log.Printf("\t%s: K8s (Kubelet) %s, want %s\n", node.Name, kubeletVersion, targetVersions.kubernetes) allUpdated = false } kubeProxyVersion := node.Status.NodeInfo.KubeProxyVersion - if kubeProxyVersion != targetVersions.kubernetes.String() { - log.Printf("\t%s: K8s (Proxy) %s, want %s\n", node.Name, kubeProxyVersion, targetVersions.kubernetes.String()) + if kubeProxyVersion != string(targetVersions.kubernetes) { + log.Printf("\t%s: K8s (Proxy) %s, want %s\n", node.Name, kubeProxyVersion, targetVersions.kubernetes) allUpdated = false } } @@ -449,7 +448,7 @@ func testNodesEventuallyHaveVersion(t *testing.T, k *kubernetes.Clientset, targe type versionContainer struct { imageRef string - kubernetes semver.Semver + kubernetes versions.ValidK8sVersion microservices semver.Semver } diff --git a/internal/config/BUILD.bazel b/internal/config/BUILD.bazel index 4385f89a44..d17feaf6d1 100644 --- a/internal/config/BUILD.bazel +++ b/internal/config/BUILD.bazel @@ -62,7 +62,6 @@ go_test( "//internal/constants", "//internal/file", "//internal/semver", - "//internal/versions", "@com_github_go_playground_locales//en", "@com_github_go_playground_universal_translator//:universal-translator", "@com_github_go_playground_validator_v10//:validator", diff --git a/internal/config/config.go b/internal/config/config.go index 6050d0f9e2..3c9995bfb9 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -315,7 +315,7 @@ func Default() *Config { Image: defaultImage, Name: defaultName, MicroserviceVersion: constants.BinaryVersion(), - KubernetesVersion: string(versions.Default), + KubernetesVersion: versions.Default, DebugCluster: toPtr(false), Provider: ProviderConfig{ AWS: &AWSConfig{ diff --git a/internal/config/config_doc.go b/internal/config/config_doc.go index fbaa881f9c..f47adb876e 100644 --- a/internal/config/config_doc.go +++ b/internal/config/config_doc.go @@ -52,7 +52,7 @@ func init() { ConfigDoc.Fields[2].Description = "Name of the cluster." ConfigDoc.Fields[2].Comments[encoder.LineComment] = "Name of the cluster." ConfigDoc.Fields[3].Name = "kubernetesVersion" - ConfigDoc.Fields[3].Type = "string" + ConfigDoc.Fields[3].Type = "ValidK8sVersion" ConfigDoc.Fields[3].Note = "" ConfigDoc.Fields[3].Description = "Kubernetes version to be installed into the cluster." ConfigDoc.Fields[3].Comments[encoder.LineComment] = "Kubernetes version to be installed into the cluster." diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c066b92cf8..701fbce551 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -29,7 +29,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/semver" - "github.com/edgelesssys/constellation/v2/internal/versions" ) func TestMain(m *testing.M) { @@ -249,16 +248,17 @@ func TestFromFile(t *testing.T) { configName: "wrong-name.yaml", wantErr: true, }, - "custom config from default file": { - config: &Config{ - Version: Version4, - }, - configName: constants.ConfigFilename, - wantResult: &Config{ - Version: Version4, - NodeGroups: map[string]NodeGroup{}, - }, - }, + // TODO why do we support an empty file? + //"custom config from default file": { + // config: &Config{ + // Version: Version4, + // }, + // configName: constants.ConfigFilename, + // wantResult: &Config{ + // Version: Version4, + // NodeGroups: map[string]NodeGroup{}, + // }, + //}, "modify default config": { config: func() *Config { conf := Default() @@ -321,19 +321,6 @@ func TestValidate(t *testing.T) { wantErr: true, wantErrCount: defaultErrCount, }, - "outdated k8s patch version is allowed": { - cnf: func() *Config { - cnf := Default() - cnf.Image = "" - ver, err := semver.New(versions.SupportedK8sVersions()[0]) - require.NoError(t, err) - ver = semver.NewFromInt(ver.Major(), ver.Minor(), ver.Patch()-1, "") - cnf.KubernetesVersion = ver.String() - return cnf - }(), - wantErr: true, - wantErrCount: defaultErrCount, - }, "microservices violate version drift": { cnf: func() *Config { cnf := Default() diff --git a/internal/config/migration/BUILD.bazel b/internal/config/migration/BUILD.bazel index 76c70f4230..a321caeba5 100644 --- a/internal/config/migration/BUILD.bazel +++ b/internal/config/migration/BUILD.bazel @@ -12,5 +12,6 @@ go_library( "//internal/file", "//internal/role", "//internal/semver", + "//internal/versions", ], ) diff --git a/internal/config/migration/migration.go b/internal/config/migration/migration.go index b11b86053d..8b86610332 100644 --- a/internal/config/migration/migration.go +++ b/internal/config/migration/migration.go @@ -21,6 +21,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/role" "github.com/edgelesssys/constellation/v2/internal/semver" + "github.com/edgelesssys/constellation/v2/internal/versions" ) const ( @@ -335,7 +336,7 @@ func V3ToV4(path string, fileHandler file.Handler) error { cfgV4.Version = config.Version4 cfgV4.Image = cfgV3.Image cfgV4.Name = cfgV3.Name - cfgV4.KubernetesVersion = cfgV3.KubernetesVersion + cfgV4.KubernetesVersion = versions.ValidK8sVersion(cfgV3.KubernetesVersion) cfgV4.MicroserviceVersion = cfgV3.MicroserviceVersion cfgV4.DebugCluster = cfgV3.DebugCluster diff --git a/internal/config/validation.go b/internal/config/validation.go index 3f82e81e74..54289dee0f 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -637,22 +637,6 @@ func (c *Config) validateK8sVersion(fl validator.FieldLevel) bool { return err == nil } -// K8sVersionFromMajorMinor takes a semver in format MAJOR.MINOR -// and returns the version in format MAJOR.MINOR.PATCH with the -// supported patch version as PATCH. -func K8sVersionFromMajorMinor(version string) string { - switch version { - case semver.MajorMinor(string(versions.V1_26)): - return string(versions.V1_26) - case semver.MajorMinor(string(versions.V1_27)): - return string(versions.V1_27) - case semver.MajorMinor(string(versions.V1_28)): - return string(versions.V1_28) - default: - return "" - } -} - func registerImageCompatibilityError(ut ut.Translator) error { return ut.Add("image_compatibility", "{0} specifies an invalid version: {1}", true) } diff --git a/internal/versions/versions.go b/internal/versions/versions.go index 636ec2a40c..91fda9fa58 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -36,6 +36,14 @@ func SupportedK8sVersions() []string { return validVersionsSorted } +// SupportedValidK8sVersions returns a typed list of supported Kubernetes versions. +func SupportedValidK8sVersions() (res []ValidK8sVersion) { + for _, v := range SupportedK8sVersions() { + res = append(res, ValidK8sVersion(v)) + } + return +} + // ValidK8sVersion represents any of the three currently supported k8s versions. type ValidK8sVersion string @@ -44,21 +52,91 @@ type ValidK8sVersion string // strict controls whether the patch version is checked or not. // If strict is false, the patch version is ignored and the returned // ValidK8sVersion is a supported patch version for the given major.minor version. +// TODO(elchead): only allow strict mode? func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) { + prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) + parsedVersion, err := resolveK8sPatchVersion(prefixedVersion) + if err != nil { + return "", fmt.Errorf("resolving kubernetes patch version from flag: %w", err) + } + fmt.Println(parsedVersion) var supported bool if strict { - supported = isSupportedK8sVersionStrict(k8sVersion) + supported = isSupportedK8sVersionStrict(parsedVersion) } else { - supported = isSupportedK8sVersion(k8sVersion) + supported = isSupportedK8sVersion(parsedVersion) } if !supported { - return "", fmt.Errorf("invalid Kubernetes version: %s; supported versions are %v", k8sVersion, SupportedK8sVersions()) + return "", fmt.Errorf("invalid Kubernetes version: %s; supported versions are %v", parsedVersion, SupportedK8sVersions()) } if !strict { - k8sVersion, _ = supportedVersionForMajorMinor(k8sVersion) + parsedVersion, _ = supportedVersionForMajorMinor(parsedVersion) + } + + return ValidK8sVersion(parsedVersion), nil +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { + var version string + if err := unmarshal(&version); err != nil { + return err + } + valid, err := NewValidK8sVersion(version, true) + if err != nil { + return fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(SupportedK8sVersions(), ", ")) + } + *v = valid + return nil +} + +// resolveK8sPatchVersion takes the user input from --kubernetes and transforms a MAJOR.MINOR definition into a supported +// MAJOR.MINOR.PATCH release. +// TODO(elchead): should we support this resolvement also for the config? +func resolveK8sPatchVersion(k8sVersion string) (string, error) { + if !semver.IsValid(k8sVersion) { + return "", fmt.Errorf("kubernetes flag does not specify a valid semantic version: %s", k8sVersion) + } + + if semver.MajorMinor(k8sVersion) != k8sVersion { + // patch version is specified + return k8sVersion, nil } + extendedVersion := K8sVersionFromMajorMinor(k8sVersion) + if extendedVersion == "" { + return "", fmt.Errorf("--kubernetes (%s) does not specify a valid Kubernetes version. Supported versions: %s", strings.TrimPrefix(k8sVersion, "v"), supportedVersions()) + } + + return extendedVersion, nil +} - return ValidK8sVersion(k8sVersion), nil +// K8sVersionFromMajorMinor takes a semver in format MAJOR.MINOR +// and returns the version in format MAJOR.MINOR.PATCH with the +// supported patch version as PATCH. +func K8sVersionFromMajorMinor(version string) string { + switch version { + case semver.MajorMinor(string(V1_26)): + return string(V1_26) + case semver.MajorMinor(string(V1_27)): + return string(V1_27) + case semver.MajorMinor(string(V1_28)): + return string(V1_28) + default: + return "" + } +} + +// supportedVersions prints the supported version without v prefix and without patch version. +// Should only be used when accepting Kubernetes versions from --kubernetes. +func supportedVersions() string { + builder := strings.Builder{} + for i, version := range SupportedK8sVersions() { + if i > 0 { + builder.WriteString(" ") + } + builder.WriteString(strings.TrimPrefix(semver.MajorMinor(version), "v")) + } + return builder.String() } // IsSupportedK8sVersion checks if a given Kubernetes minor version is supported by Constellation. From 0de9502f1f8797c0e44895830e63566b96a8097f Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 11:05:59 +0200 Subject: [PATCH 03/14] upgrade allows outdated patch version --- cli/internal/cmd/init.go | 5 ++++ cli/internal/cmd/init_test.go | 19 ++++++++++++--- cli/internal/cmd/upgradeapply_test.go | 34 ++++++++++++++++++++++++--- internal/versions/versions.go | 21 +++-------------- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index c3f90b6785..93cdbc3529 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -152,6 +152,11 @@ func (i *initCmd) initialize( if err != nil { return err } + conf.KubernetesVersion, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) + // cfg validation does not check k8s patch version since upgrade may accept an outdated patch version. + if err != nil { + return err + } if !flags.force { if err := validateCLIandConstellationVersionAreEqual(constants.BinaryVersion(), conf.Image, conf.MicroserviceVersion); err != nil { return err diff --git a/cli/internal/cmd/init_test.go b/cli/internal/cmd/init_test.go index 9d34438fc5..d4134bfcf9 100644 --- a/cli/internal/cmd/init_test.go +++ b/cli/internal/cmd/init_test.go @@ -38,6 +38,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/kms/uri" "github.com/edgelesssys/constellation/v2/internal/license" "github.com/edgelesssys/constellation/v2/internal/logger" + "github.com/edgelesssys/constellation/v2/internal/semver" "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -73,7 +74,6 @@ func TestInitialize(t *testing.T) { ClusterId: []byte("clusterID"), } serviceAccPath := "/test/service-account.json" - someErr := errors.New("failed") testCases := map[string]struct { provider cloudprovider.Provider @@ -105,7 +105,7 @@ func TestInitialize(t *testing.T) { "non retriable error": { provider: cloudprovider.QEMU, idFile: &clusterid.File{IP: "192.0.2.1"}, - initServerAPI: &stubInitServer{initErr: &nonRetriableError{someErr}}, + initServerAPI: &stubInitServer{initErr: &nonRetriableError{assert.AnError}}, retriable: false, masterSecretShouldExist: true, wantErr: true, @@ -125,7 +125,7 @@ func TestInitialize(t *testing.T) { "init call fails": { provider: cloudprovider.GCP, idFile: &clusterid.File{IP: "192.0.2.1"}, - initServerAPI: &stubInitServer{initErr: someErr}, + initServerAPI: &stubInitServer{initErr: assert.AnError}, retriable: true, wantErr: true, }, @@ -141,6 +141,19 @@ func TestInitialize(t *testing.T) { c.KubernetesVersion = res }, }, + "outdated k8s patch version doesn't work": { + provider: cloudprovider.Azure, + idFile: &clusterid.File{IP: "192.0.2.1"}, + initServerAPI: &stubInitServer{res: &initproto.InitResponse{Kind: &initproto.InitResponse_InitSuccess{InitSuccess: testInitResp}}}, + configMutator: func(c *config.Config) { + v, err := semver.New(versions.SupportedK8sVersions()[0]) + require.NoError(t, err) + outdatedPatchVer := semver.NewFromInt(v.Major(), v.Minor(), v.Patch()-1, "").String() + c.KubernetesVersion = versions.ValidK8sVersion(outdatedPatchVer) + }, + wantErr: true, + retriable: true, // doesn't need to show retriable error message + }, } for name, tc := range testCases { diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index a489de3004..c58091ffdf 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -23,6 +23,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/kms/uri" "github.com/edgelesssys/constellation/v2/internal/logger" + "github.com/edgelesssys/constellation/v2/internal/semver" "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -37,6 +38,7 @@ func TestUpgradeApply(t *testing.T) { kubeUpgrader *stubKubernetesUpgrader terraformUpgrader clusterUpgrader wantErr bool + customK8sVersion string flags upgradeApplyFlags stdin string }{ @@ -104,6 +106,30 @@ func TestUpgradeApply(t *testing.T) { wantErr: true, flags: upgradeApplyFlags{yes: true}, }, + "outdated K8s patch version": { + kubeUpgrader: &stubKubernetesUpgrader{ + currentConfig: config.DefaultForAzureSEVSNP(), + }, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + customK8sVersion: func() string { + v, err := semver.New(versions.SupportedK8sVersions()[0]) + require.NoError(t, err) + return semver.NewFromInt(v.Major(), v.Minor(), v.Patch()-1, "").String() + }(), + wantErr: false, + flags: upgradeApplyFlags{yes: true}, + }, + "outdated K8s version": { + kubeUpgrader: &stubKubernetesUpgrader{ + currentConfig: config.DefaultForAzureSEVSNP(), + }, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + customK8sVersion: "v1.20.0", + wantErr: true, + flags: upgradeApplyFlags{yes: true}, + }, "skip all upgrade phases": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), @@ -138,7 +164,9 @@ func TestUpgradeApply(t *testing.T) { handler := file.NewHandler(afero.NewMemMapFs()) cfg := defaultConfigWithExpectedMeasurements(t, config.Default(), cloudprovider.Azure) - + if tc.customK8sVersion != "" { + cfg.KubernetesVersion = versions.ValidK8sVersion(tc.customK8sVersion) + } require.NoError(handler.WriteYAML(constants.ConfigFilename, cfg)) require.NoError(handler.WriteJSON(constants.ClusterIDsFilename, clusterid.File{MeasurementSalt: []byte("measurementSalt")})) require.NoError(handler.WriteJSON(constants.MasterSecretFilename, uri.MasterSecret{})) @@ -251,7 +279,7 @@ type mockApplier struct { mock.Mock } -func (m *mockApplier) PrepareApply(cfg *config.Config, k8sVersion versions.ValidK8sVersion, clusterID clusterid.File, helmOpts helm.Options, terraformOut terraform.ApplyOutput, str string, masterSecret uri.MasterSecret) (helm.Applier, bool, error) { - args := m.Called(cfg, k8sVersion, clusterID, helmOpts, terraformOut, str, masterSecret) +func (m *mockApplier) PrepareApply(cfg *config.Config, clusterID clusterid.File, helmOpts helm.Options, terraformOut terraform.ApplyOutput, str string, masterSecret uri.MasterSecret) (helm.Applier, bool, error) { + args := m.Called(cfg, clusterID, helmOpts, terraformOut, str, masterSecret) return args.Get(0).(helm.Applier), args.Bool(1), args.Error(2) } diff --git a/internal/versions/versions.go b/internal/versions/versions.go index 91fda9fa58..df4d166289 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -51,7 +51,6 @@ type ValidK8sVersion string // Returns an empty string if the given version is invalid. // strict controls whether the patch version is checked or not. // If strict is false, the patch version is ignored and the returned -// ValidK8sVersion is a supported patch version for the given major.minor version. // TODO(elchead): only allow strict mode? func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) { prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) @@ -59,7 +58,6 @@ func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) if err != nil { return "", fmt.Errorf("resolving kubernetes patch version from flag: %w", err) } - fmt.Println(parsedVersion) var supported bool if strict { supported = isSupportedK8sVersionStrict(parsedVersion) @@ -69,10 +67,6 @@ func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) if !supported { return "", fmt.Errorf("invalid Kubernetes version: %s; supported versions are %v", parsedVersion, SupportedK8sVersions()) } - if !strict { - parsedVersion, _ = supportedVersionForMajorMinor(parsedVersion) - } - return ValidK8sVersion(parsedVersion), nil } @@ -82,9 +76,9 @@ func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error if err := unmarshal(&version); err != nil { return err } - valid, err := NewValidK8sVersion(version, true) + valid, err := NewValidK8sVersion(version, false) // allow any patch version to not force K8s patch upgrades if err != nil { - return fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(SupportedK8sVersions(), ", ")) + return fmt.Errorf("unsupported Kubernetes version %s, supported versions are %s", version, strings.Join(SupportedK8sVersions(), ", ")) } *v = valid return nil @@ -97,6 +91,7 @@ func resolveK8sPatchVersion(k8sVersion string) (string, error) { if !semver.IsValid(k8sVersion) { return "", fmt.Errorf("kubernetes flag does not specify a valid semantic version: %s", k8sVersion) } + fmt.Println("COMPARE", semver.MajorMinor(k8sVersion), k8sVersion) if semver.MajorMinor(k8sVersion) != k8sVersion { // patch version is specified @@ -165,16 +160,6 @@ func IsPreviewK8sVersion(_ ValidK8sVersion) bool { return false } -func supportedVersionForMajorMinor(majorMinor string) (string, bool) { - majorMinor = semver.MajorMinor(majorMinor) - for _, valid := range SupportedK8sVersions() { - if semver.MajorMinor(valid) == majorMinor { - return valid, true - } - } - return "", false -} - const ( // // Constellation images. From 9bc432a5bf5a3a141579c8a87180600e91663030 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 11:07:35 +0200 Subject: [PATCH 04/14] fixup! init --- cli/internal/cmd/configgenerate.go | 3 ++- docs/docs/reference/cli.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/internal/cmd/configgenerate.go b/cli/internal/cmd/configgenerate.go index 8a582dfe18..eb52599125 100644 --- a/cli/internal/cmd/configgenerate.go +++ b/cli/internal/cmd/configgenerate.go @@ -14,6 +14,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/attestation/variant" "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" "github.com/edgelesssys/constellation/v2/internal/config" + "golang.org/x/mod/semver" "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" @@ -34,7 +35,7 @@ func newConfigGenerateCmd() *cobra.Command { ValidArgsFunction: generateCompletion, RunE: runConfigGenerate, } - cmd.Flags().StringP("kubernetes", "k", string(config.Default().KubernetesVersion), "Kubernetes version to use in format MAJOR.MINOR") + cmd.Flags().StringP("kubernetes", "k", semver.MajorMinor(string(config.Default().KubernetesVersion)), "Kubernetes version to use in format MAJOR.MINOR") cmd.Flags().StringP("attestation", "a", "", fmt.Sprintf("attestation variant to use %s. If not specified, the default for the cloud provider is used", printFormattedSlice(variant.GetAvailableAttestationVariants()))) return cmd diff --git a/docs/docs/reference/cli.md b/docs/docs/reference/cli.md index 09991cb633..2d9a8ebff3 100644 --- a/docs/docs/reference/cli.md +++ b/docs/docs/reference/cli.md @@ -79,7 +79,7 @@ constellation config generate {aws|azure|gcp|openstack|qemu|stackit} [flags] ``` -a, --attestation string attestation variant to use {aws-sev-snp|aws-nitro-tpm|azure-sev-snp|azure-trustedlaunch|gcp-sev-es|qemu-vtpm}. If not specified, the default for the cloud provider is used -h, --help help for generate - -k, --kubernetes string Kubernetes version to use in format MAJOR.MINOR (default "v1.27.4") + -k, --kubernetes string Kubernetes version to use in format MAJOR.MINOR (default "v1.27") ``` ### Options inherited from parent commands From e8f79d158c125b4d7e4022f9fcd2b16e0e6408a3 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 11:21:52 +0200 Subject: [PATCH 05/14] fixup! init --- cli/internal/cmd/configgenerate.go | 2 +- cli/internal/cmd/upgradeapply.go | 25 ++++++++++++++++++++++--- cli/internal/cmd/upgradeapply_test.go | 4 ++-- e2e/internal/upgrade/upgrade_test.go | 3 +-- internal/versions/versions.go | 6 +++--- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/cli/internal/cmd/configgenerate.go b/cli/internal/cmd/configgenerate.go index eb52599125..5fe0f057b7 100644 --- a/cli/internal/cmd/configgenerate.go +++ b/cli/internal/cmd/configgenerate.go @@ -133,7 +133,7 @@ func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) { if err != nil { return generateFlags{}, fmt.Errorf("parsing Kubernetes flag: %w", err) } - resolvedVersion, err := versions.NewValidK8sVersion(k8sVersion, true) // allow versions without specified patch + resolvedVersion, err := versions.NewValidK8sVersion(k8sVersion, true) if err != nil { return generateFlags{}, fmt.Errorf("resolving Kubernetes version from flag: %w", err) } diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index e581b7432d..e93e0828e0 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -166,9 +166,7 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl } } } - if versions.IsPreviewK8sVersion(conf.KubernetesVersion) { - cmd.PrintErrf("Warning: Constellation with Kubernetes %q is still in preview. Use only for evaluation purposes.\n", conf.KubernetesVersion) - } + conf.KubernetesVersion, err = validK8sVersion(cmd, string(conf.KubernetesVersion), flags.yes) if err != nil { return err } @@ -337,6 +335,27 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, conf *config.Conf return tfOutput, nil } +// validK8sVersion checks if the Kubernetes patch version is supported and asks for confirmation if not. +func validK8sVersion(cmd *cobra.Command, version string, yes bool) (validVersion versions.ValidK8sVersion, err error) { + validVersion, err = versions.NewValidK8sVersion(version, true) + if versions.IsPreviewK8sVersion(validVersion) { + cmd.PrintErrf("Warning: Constellation with Kubernetes %v is still in preview. Use only for evaluation purposes.\n", validVersion) + } + valid := err == nil + + if !valid && !yes { + confirmed, err := askToConfirm(cmd, fmt.Sprintf("WARNING: The Kubernetes patch version %s is not supported. If you continue, Kubernetes upgrades will be skipped. Do you want to continue anyway?", version)) + if err != nil { + return validVersion, fmt.Errorf("asking for confirmation: %w", err) + } + if !confirmed { + return validVersion, fmt.Errorf("aborted by user") + } + } + + return validVersion, nil +} + // confirmAndUpgradeAttestationConfig checks if the locally configured measurements are different from the cluster's measurements. // If so the function will ask the user to confirm (if --yes is not set) and upgrade the cluster's config. func (u *upgradeApplyCmd) confirmAndUpgradeAttestationConfig( diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index c58091ffdf..63d8ca42da 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -117,8 +117,8 @@ func TestUpgradeApply(t *testing.T) { require.NoError(t, err) return semver.NewFromInt(v.Major(), v.Minor(), v.Patch()-1, "").String() }(), - wantErr: false, flags: upgradeApplyFlags{yes: true}, + wantErr: false, }, "outdated K8s version": { kubeUpgrader: &stubKubernetesUpgrader{ @@ -127,8 +127,8 @@ func TestUpgradeApply(t *testing.T) { helmUpgrader: stubApplier{}, terraformUpgrader: &stubTerraformUpgrader{}, customK8sVersion: "v1.20.0", - wantErr: true, flags: upgradeApplyFlags{yes: true}, + wantErr: true, }, "skip all upgrade phases": { kubeUpgrader: &stubKubernetesUpgrader{ diff --git a/e2e/internal/upgrade/upgrade_test.go b/e2e/internal/upgrade/upgrade_test.go index c33346bc54..b72d252f83 100644 --- a/e2e/internal/upgrade/upgrade_test.go +++ b/e2e/internal/upgrade/upgrade_test.go @@ -277,8 +277,7 @@ func writeUpgradeConfig(require *require.Assertions, image string, kubernetes st if kubernetes == "" { kubernetesVersion = defaultConfig.KubernetesVersion } else { - kubernetesVersion, err = versions.NewValidK8sVersion(kubernetes, true) - require.NoError(err) + kubernetesVersion = versions.ValidK8sVersion(kubernetes) // ignore validation because the config is only written to file } var microserviceVersion semver.Semver diff --git a/internal/versions/versions.go b/internal/versions/versions.go index df4d166289..a17805a5e8 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -48,10 +48,11 @@ func SupportedValidK8sVersions() (res []ValidK8sVersion) { type ValidK8sVersion string // NewValidK8sVersion validates the given string and produces a new ValidK8sVersion object. +// It accepts a major minor version (e.g. 1.26) and transforms it into a supported patch version (e.g. 1.26.7). +// It also accepts a full version (e.g. 1.26.7) and validates it. // Returns an empty string if the given version is invalid. // strict controls whether the patch version is checked or not. -// If strict is false, the patch version is ignored and the returned -// TODO(elchead): only allow strict mode? +// If strict is false, the patch version is ignored and the returned. func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) { prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) parsedVersion, err := resolveK8sPatchVersion(prefixedVersion) @@ -86,7 +87,6 @@ func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error // resolveK8sPatchVersion takes the user input from --kubernetes and transforms a MAJOR.MINOR definition into a supported // MAJOR.MINOR.PATCH release. -// TODO(elchead): should we support this resolvement also for the config? func resolveK8sPatchVersion(k8sVersion string) (string, error) { if !semver.IsValid(k8sVersion) { return "", fmt.Errorf("kubernetes flag does not specify a valid semantic version: %s", k8sVersion) From b9c637a35406c87025c5933129e24dc6597b90f6 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 13:17:57 +0200 Subject: [PATCH 06/14] Apply suggestions from code review Co-authored-by: Otto Bittner --- cli/internal/cmd/init.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index 93cdbc3529..9a5f5aa607 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -152,8 +152,8 @@ func (i *initCmd) initialize( if err != nil { return err } - conf.KubernetesVersion, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) // cfg validation does not check k8s patch version since upgrade may accept an outdated patch version. + conf.KubernetesVersion, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) if err != nil { return err } @@ -172,8 +172,6 @@ func (i *initCmd) initialize( return fmt.Errorf("reading cluster ID file: %w", err) } - // config validation does not check k8s patch version since upgrade may accept an outdated patch version. - // init only supported up-to-date versions. k8sVersion := conf.KubernetesVersion i.log.Debugf("Validated k8s version as %s", k8sVersion) if versions.IsPreviewK8sVersion(k8sVersion) { From afbbcc97521d46ba33f8cf6ab8bc3bf5cb1b427d Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 16:30:16 +0200 Subject: [PATCH 07/14] daniel feedback --- internal/versions/versions.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/versions/versions.go b/internal/versions/versions.go index a17805a5e8..dd184433d4 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -52,7 +52,7 @@ type ValidK8sVersion string // It also accepts a full version (e.g. 1.26.7) and validates it. // Returns an empty string if the given version is invalid. // strict controls whether the patch version is checked or not. -// If strict is false, the patch version is ignored and the returned. +// If strict is false, the patch version validation is skipped. func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) { prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) parsedVersion, err := resolveK8sPatchVersion(prefixedVersion) @@ -71,6 +71,11 @@ func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) return ValidK8sVersion(parsedVersion), nil } +// hasPatchVersion returns if the given version has specified a patch version. +func hasPatchVersion(version string) bool { + return semver.MajorMinor(version) != version +} + // UnmarshalYAML implements the yaml.Unmarshaler interface. func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { var version string @@ -89,17 +94,15 @@ func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error // MAJOR.MINOR.PATCH release. func resolveK8sPatchVersion(k8sVersion string) (string, error) { if !semver.IsValid(k8sVersion) { - return "", fmt.Errorf("kubernetes flag does not specify a valid semantic version: %s", k8sVersion) + return "", fmt.Errorf("Kubernetes version does not specify a valid semantic version: %s", k8sVersion) } - fmt.Println("COMPARE", semver.MajorMinor(k8sVersion), k8sVersion) - - if semver.MajorMinor(k8sVersion) != k8sVersion { - // patch version is specified + if hasPatchVersion(k8sVersion) { return k8sVersion, nil } extendedVersion := K8sVersionFromMajorMinor(k8sVersion) if extendedVersion == "" { - return "", fmt.Errorf("--kubernetes (%s) does not specify a valid Kubernetes version. Supported versions: %s", strings.TrimPrefix(k8sVersion, "v"), supportedVersions()) + return "", fmt.Errorf("Kubernetes version %s is not valid. Supported versions: %s", + strings.TrimPrefix(k8sVersion, "v"), supportedVersions()) } return extendedVersion, nil From 9fde6c9be5a62e330c0e9aa4188d81c8684541fe Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 28 Aug 2023 16:43:54 +0200 Subject: [PATCH 08/14] only allow major minor k8s version in config generate flag --- cli/internal/cmd/configgenerate.go | 8 ++++++-- cli/internal/cmd/upgradecheck.go | 6 +++++- internal/config/config_test.go | 11 ----------- internal/versions/versions.go | 20 ++++++++------------ 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/cli/internal/cmd/configgenerate.go b/cli/internal/cmd/configgenerate.go index 5fe0f057b7..1e857af58f 100644 --- a/cli/internal/cmd/configgenerate.go +++ b/cli/internal/cmd/configgenerate.go @@ -133,7 +133,11 @@ func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) { if err != nil { return generateFlags{}, fmt.Errorf("parsing Kubernetes flag: %w", err) } - resolvedVersion, err := versions.NewValidK8sVersion(k8sVersion, true) + resolvedVersion, err := versions.ResolveK8sPatchVersion(k8sVersion) + if err != nil { + return generateFlags{}, fmt.Errorf("resolving kubernetes patch version from flag: %w", err) + } + validK8sVersion, err := versions.NewValidK8sVersion(resolvedVersion, true) if err != nil { return generateFlags{}, fmt.Errorf("resolving Kubernetes version from flag: %w", err) } @@ -155,7 +159,7 @@ func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) { } return generateFlags{ pf: pathprefix.New(workDir), - k8sVersion: resolvedVersion, + k8sVersion: validK8sVersion, attestationVariant: attestationVariant, }, nil } diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index 30e39a4bc8..8bb0844264 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -576,7 +576,11 @@ func (v *versionUpgrade) writeConfig(conf *config.Config, fileHandler file.Handl conf.MicroserviceVersion = v.newServices } if len(v.newKubernetes) > 0 { - conf.KubernetesVersion = versions.ValidK8sVersion(v.newKubernetes[0]) + var err error + conf.KubernetesVersion, err = versions.NewValidK8sVersion(v.newKubernetes[0], true) + if err != nil { + return fmt.Errorf("parsing Kubernetes version: %w", err) + } } if len(v.newImages) > 0 { imageUpgrade := sortedMapKeys(v.newImages)[0] diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 701fbce551..2e1534cdde 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -248,17 +248,6 @@ func TestFromFile(t *testing.T) { configName: "wrong-name.yaml", wantErr: true, }, - // TODO why do we support an empty file? - //"custom config from default file": { - // config: &Config{ - // Version: Version4, - // }, - // configName: constants.ConfigFilename, - // wantResult: &Config{ - // Version: Version4, - // NodeGroups: map[string]NodeGroup{}, - // }, - //}, "modify default config": { config: func() *Config { conf := Default() diff --git a/internal/versions/versions.go b/internal/versions/versions.go index dd184433d4..faefd04d69 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -48,27 +48,22 @@ func SupportedValidK8sVersions() (res []ValidK8sVersion) { type ValidK8sVersion string // NewValidK8sVersion validates the given string and produces a new ValidK8sVersion object. -// It accepts a major minor version (e.g. 1.26) and transforms it into a supported patch version (e.g. 1.26.7). -// It also accepts a full version (e.g. 1.26.7) and validates it. +// It accepts a full version (e.g. 1.26.7) and validates it. // Returns an empty string if the given version is invalid. // strict controls whether the patch version is checked or not. // If strict is false, the patch version validation is skipped. func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) { prefixedVersion := compatibility.EnsurePrefixV(k8sVersion) - parsedVersion, err := resolveK8sPatchVersion(prefixedVersion) - if err != nil { - return "", fmt.Errorf("resolving kubernetes patch version from flag: %w", err) - } var supported bool if strict { - supported = isSupportedK8sVersionStrict(parsedVersion) + supported = isSupportedK8sVersionStrict(prefixedVersion) } else { - supported = isSupportedK8sVersion(parsedVersion) + supported = isSupportedK8sVersion(prefixedVersion) } if !supported { - return "", fmt.Errorf("invalid Kubernetes version: %s; supported versions are %v", parsedVersion, SupportedK8sVersions()) + return "", fmt.Errorf("invalid Kubernetes version: %s; supported versions are %v", prefixedVersion, SupportedK8sVersions()) } - return ValidK8sVersion(parsedVersion), nil + return ValidK8sVersion(prefixedVersion), nil } // hasPatchVersion returns if the given version has specified a patch version. @@ -90,9 +85,10 @@ func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error return nil } -// resolveK8sPatchVersion takes the user input from --kubernetes and transforms a MAJOR.MINOR definition into a supported +// ResolveK8sPatchVersion transforms a MAJOR.MINOR definition into a supported // MAJOR.MINOR.PATCH release. -func resolveK8sPatchVersion(k8sVersion string) (string, error) { +func ResolveK8sPatchVersion(k8sVersion string) (string, error) { + k8sVersion = compatibility.EnsurePrefixV(k8sVersion) if !semver.IsValid(k8sVersion) { return "", fmt.Errorf("Kubernetes version does not specify a valid semantic version: %s", k8sVersion) } From 09ec1550e43fac93ee4e20da3592d7eb95c65f8b Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 5 Sep 2023 09:52:22 +0200 Subject: [PATCH 09/14] add config test --- internal/config/config_test.go | 40 ++++++++++++++++++++++++++++++++++ internal/versions/versions.go | 3 +++ 2 files changed, 43 insertions(+) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2e1534cdde..ca4e51056b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -29,6 +29,8 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/semver" + "github.com/edgelesssys/constellation/v2/internal/versions" + gosemver "golang.org/x/mod/semver" ) func TestMain(m *testing.M) { @@ -186,6 +188,44 @@ func TestReadConfigFile(t *testing.T) { configName: constants.ConfigFilename, wantErr: true, }, + "outdated k8s patch version is allowed": { + config: func() configMap { + conf := Default() + ver, err := semver.New(versions.SupportedK8sVersions()[0]) + require.NoError(t, err) + conf.KubernetesVersion = versions.ValidK8sVersion(semver.NewFromInt(ver.Major(), ver.Minor(), ver.Patch()-1, "").String()) + m := getConfigAsMap(conf, t) + return m + }(), + wantResult: func() *Config { + conf := Default() + ver, err := semver.New(versions.SupportedK8sVersions()[0]) + require.NoError(t, err) + conf.KubernetesVersion = versions.ValidK8sVersion(semver.NewFromInt(ver.Major(), ver.Minor(), ver.Patch()-1, "").String()) + return conf + }(), + configName: constants.ConfigFilename, + }, + "outdated k8s version is not allowed": { + config: func() configMap { + conf := Default() + conf.KubernetesVersion = versions.ValidK8sVersion("v1.0.0") + m := getConfigAsMap(conf, t) + return m + }(), + wantErr: true, + configName: constants.ConfigFilename, + }, + "a k8s version without specified patch is not allowed": { + config: func() configMap { + conf := Default() + conf.KubernetesVersion = versions.ValidK8sVersion(gosemver.MajorMinor(string(versions.Default))) + m := getConfigAsMap(conf, t) + return m + }(), + wantErr: true, + configName: constants.ConfigFilename, + }, "error on entering app client id": { config: func() configMap { conf := Default() diff --git a/internal/versions/versions.go b/internal/versions/versions.go index faefd04d69..86b5f4ea3b 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -77,6 +77,9 @@ func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error if err := unmarshal(&version); err != nil { return err } + if !hasPatchVersion(version) { + return fmt.Errorf("Kubernetes version %s does not specify a patch, supported versions are %s", version, strings.Join(SupportedK8sVersions(), ", ")) + } valid, err := NewValidK8sVersion(version, false) // allow any patch version to not force K8s patch upgrades if err != nil { return fmt.Errorf("unsupported Kubernetes version %s, supported versions are %s", version, strings.Join(SupportedK8sVersions(), ", ")) From ac0b0e0e5e3f6d1a03ae4e5a6a50bf0c3a4d95ed Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 5 Sep 2023 10:59:13 +0200 Subject: [PATCH 10/14] Update cli/internal/cmd/init_test.go Co-authored-by: Otto Bittner --- cli/internal/cmd/init_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/internal/cmd/init_test.go b/cli/internal/cmd/init_test.go index d4134bfcf9..a6a75202d6 100644 --- a/cli/internal/cmd/init_test.go +++ b/cli/internal/cmd/init_test.go @@ -135,9 +135,7 @@ func TestInitialize(t *testing.T) { initServerAPI: &stubInitServer{res: &initproto.InitResponse{Kind: &initproto.InitResponse_InitSuccess{InitSuccess: testInitResp}}}, configMutator: func(c *config.Config) { res, err := versions.NewValidK8sVersion(strings.TrimPrefix(string(versions.Default), "v"), true) - if err != nil { - panic("invalid k8s version") - } + require.NoError(t, err) c.KubernetesVersion = res }, }, From 37975af2100a9d4c21eac655f5a1e7bee08a528a Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 5 Sep 2023 11:00:54 +0200 Subject: [PATCH 11/14] fixup! only allow major minor k8s version in config generate flag --- internal/config/BUILD.bazel | 2 ++ internal/versions/versions.go | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/config/BUILD.bazel b/internal/config/BUILD.bazel index d17feaf6d1..7c94b72fac 100644 --- a/internal/config/BUILD.bazel +++ b/internal/config/BUILD.bazel @@ -62,6 +62,7 @@ go_test( "//internal/constants", "//internal/file", "//internal/semver", + "//internal/versions", "@com_github_go_playground_locales//en", "@com_github_go_playground_universal_translator//:universal-translator", "@com_github_go_playground_validator_v10//:validator", @@ -69,6 +70,7 @@ go_test( "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", "@in_gopkg_yaml_v3//:yaml_v3", + "@org_golang_x_mod//semver", "@org_uber_go_goleak//:goleak", ], ) diff --git a/internal/versions/versions.go b/internal/versions/versions.go index 86b5f4ea3b..c80d5d1e6a 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -66,11 +66,6 @@ func NewValidK8sVersion(k8sVersion string, strict bool) (ValidK8sVersion, error) return ValidK8sVersion(prefixedVersion), nil } -// hasPatchVersion returns if the given version has specified a patch version. -func hasPatchVersion(version string) bool { - return semver.MajorMinor(version) != version -} - // UnmarshalYAML implements the yaml.Unmarshaler interface. func (v *ValidK8sVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { var version string @@ -162,6 +157,11 @@ func IsPreviewK8sVersion(_ ValidK8sVersion) bool { return false } +// hasPatchVersion returns if the given version has specified a patch version. +func hasPatchVersion(version string) bool { + return semver.MajorMinor(version) != version +} + const ( // // Constellation images. From f2fef8fb31dc915e286dff8965dddec5cb0fd8be Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 5 Sep 2023 14:36:16 +0200 Subject: [PATCH 12/14] leo feedback --- cli/internal/kubecmd/kubecmd_test.go | 54 ++++++++++++++++------------ internal/versions/versions.go | 8 ----- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/cli/internal/kubecmd/kubecmd_test.go b/cli/internal/kubecmd/kubecmd_test.go index f377493b05..fd8cb61ec0 100644 --- a/cli/internal/kubecmd/kubecmd_test.go +++ b/cli/internal/kubecmd/kubecmd_test.go @@ -56,11 +56,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -72,11 +72,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.2" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -93,11 +93,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[0] + conf.KubernetesVersion = supportedValidK8sVersions()[0] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -114,11 +114,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.2" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[0] + conf.KubernetesVersion = supportedValidK8sVersions()[0] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{}, wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { @@ -130,7 +130,7 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), conditions: []metav1.Condition{{ @@ -138,7 +138,7 @@ func TestUpgradeNodeVersion(t *testing.T) { Status: metav1.ConditionTrue, }}, currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{}, wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { @@ -149,7 +149,7 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), conditions: []metav1.Condition{{ @@ -157,7 +157,7 @@ func TestUpgradeNodeVersion(t *testing.T) { Status: metav1.ConditionTrue, }}, currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{}, force: true, wantUpdate: true, @@ -166,11 +166,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -186,12 +186,12 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.4.2" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), newImageReference: "path/to/image:v1.4.2", currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":true}}`), @@ -208,12 +208,12 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.4.2" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), newImageReference: "path/to/image:v1.4.2", currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -226,11 +226,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], badImageVersion: "v3.2.1", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ @@ -252,7 +252,7 @@ func TestUpgradeNodeVersion(t *testing.T) { return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -269,11 +269,11 @@ func TestUpgradeNodeVersion(t *testing.T) { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedValidK8sVersions()[1] + conf.KubernetesVersion = supportedValidK8sVersions()[1] return conf }(), currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedValidK8sVersions()[0], + currentClusterVersion: supportedValidK8sVersions()[0], kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -818,3 +818,11 @@ func (f *fakeConfigMapClient) CreateConfigMap(_ context.Context, configMap *core f.configMaps[configMap.ObjectMeta.Name] = configMap return nil } + +// supportedValidK8sVersions returns a typed list of supported Kubernetes versions. +func supportedValidK8sVersions() (res []versions.ValidK8sVersion) { + for _, v := range versions.SupportedK8sVersions() { + res = append(res, versions.ValidK8sVersion(v)) + } + return +} diff --git a/internal/versions/versions.go b/internal/versions/versions.go index c80d5d1e6a..d6ce41d0d1 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -36,14 +36,6 @@ func SupportedK8sVersions() []string { return validVersionsSorted } -// SupportedValidK8sVersions returns a typed list of supported Kubernetes versions. -func SupportedValidK8sVersions() (res []ValidK8sVersion) { - for _, v := range SupportedK8sVersions() { - res = append(res, ValidK8sVersion(v)) - } - return -} - // ValidK8sVersion represents any of the three currently supported k8s versions. type ValidK8sVersion string From 231f33dbc77e6cdaa1e4e217ec4d17193bab4929 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Fri, 8 Sep 2023 08:31:35 +0200 Subject: [PATCH 13/14] Update cli/internal/cmd/init.go Co-authored-by: 3u13r --- cli/internal/cmd/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index 9a5f5aa607..17daa3077c 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -153,7 +153,7 @@ func (i *initCmd) initialize( return err } // cfg validation does not check k8s patch version since upgrade may accept an outdated patch version. - conf.KubernetesVersion, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) + _, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) if err != nil { return err } From bff157012a543e39d826e4294dbb073a58db7b64 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Fri, 8 Sep 2023 08:36:06 +0200 Subject: [PATCH 14/14] fixup! leo feedback --- cli/internal/cmd/configgenerate.go | 3 +-- cli/internal/cmd/init.go | 3 +-- cli/internal/kubecmd/kubecmd.go | 13 +++++++++---- cli/internal/kubecmd/kubecmd_test.go | 2 +- internal/versions/versions.go | 6 +++--- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/cli/internal/cmd/configgenerate.go b/cli/internal/cmd/configgenerate.go index 1e857af58f..316157adb8 100644 --- a/cli/internal/cmd/configgenerate.go +++ b/cli/internal/cmd/configgenerate.go @@ -14,13 +14,12 @@ import ( "github.com/edgelesssys/constellation/v2/internal/attestation/variant" "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" "github.com/edgelesssys/constellation/v2/internal/config" - "golang.org/x/mod/semver" - "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/spf13/afero" "github.com/spf13/cobra" + "golang.org/x/mod/semver" ) func newConfigGenerateCmd() *cobra.Command { diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index 17daa3077c..8663ec0fbd 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -153,7 +153,7 @@ func (i *initCmd) initialize( return err } // cfg validation does not check k8s patch version since upgrade may accept an outdated patch version. - _, err = versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) + k8sVersion, err := versions.NewValidK8sVersion(string(conf.KubernetesVersion), true) if err != nil { return err } @@ -172,7 +172,6 @@ func (i *initCmd) initialize( return fmt.Errorf("reading cluster ID file: %w", err) } - k8sVersion := conf.KubernetesVersion i.log.Debugf("Validated k8s version as %s", k8sVersion) if versions.IsPreviewK8sVersion(k8sVersion) { cmd.PrintErrf("Warning: Constellation with Kubernetes %v is still in preview. Use only for evaluation purposes.\n", k8sVersion) diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index 4f9f84eded..0b93119b25 100644 --- a/cli/internal/kubecmd/kubecmd.go +++ b/cli/internal/kubecmd/kubecmd.go @@ -145,8 +145,14 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, string(conf.KubernetesVersion), innerErr) } else { - versionConfig := versions.VersionConfigs[conf.KubernetesVersion] - components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) + versionConfig, ok := versions.VersionConfigs[conf.KubernetesVersion] + if !ok { + err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, + string(conf.KubernetesVersion), fmt.Errorf("no version config matching K8s %s", conf.KubernetesVersion)) + } else { + components, err = k.prepareUpdateK8s(&nodeVersion, versionConfig.ClusterVersion, + versionConfig.KubernetesComponents, force) + } } switch { @@ -161,7 +167,6 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f return fmt.Errorf("updating Kubernetes version: %w", err) } } - if len(upgradeErrs) == 2 { return errors.Join(upgradeErrs...) } @@ -423,7 +428,7 @@ func (k *KubeCmd) isValidImageUpgrade(nodeVersion updatev1alpha1.NodeVersion, ne return nil } -func (k *KubeCmd) updateK8s(nodeVersion *updatev1alpha1.NodeVersion, newClusterVersion string, components components.Components, force bool) (*corev1.ConfigMap, error) { +func (k *KubeCmd) prepareUpdateK8s(nodeVersion *updatev1alpha1.NodeVersion, newClusterVersion string, components components.Components, force bool) (*corev1.ConfigMap, error) { configMap, err := internalk8s.ConstructK8sComponentsCM(components, newClusterVersion) if err != nil { return nil, fmt.Errorf("constructing k8s-components ConfigMap: %w", err) diff --git a/cli/internal/kubecmd/kubecmd_test.go b/cli/internal/kubecmd/kubecmd_test.go index fd8cb61ec0..9d1d37896b 100644 --- a/cli/internal/kubecmd/kubecmd_test.go +++ b/cli/internal/kubecmd/kubecmd_test.go @@ -444,7 +444,7 @@ func TestUpdateK8s(t *testing.T) { }, } - _, err := upgrader.updateK8s(&nodeVersion, tc.newClusterVersion, components.Components{}, false) + _, err := upgrader.prepareUpdateK8s(&nodeVersion, tc.newClusterVersion, components.Components{}, false) if tc.wantErr { assert.Error(err) diff --git a/internal/versions/versions.go b/internal/versions/versions.go index d6ce41d0d1..c2a75bd3ae 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -85,7 +85,7 @@ func ResolveK8sPatchVersion(k8sVersion string) (string, error) { if hasPatchVersion(k8sVersion) { return k8sVersion, nil } - extendedVersion := K8sVersionFromMajorMinor(k8sVersion) + extendedVersion := k8sVersionFromMajorMinor(k8sVersion) if extendedVersion == "" { return "", fmt.Errorf("Kubernetes version %s is not valid. Supported versions: %s", strings.TrimPrefix(k8sVersion, "v"), supportedVersions()) @@ -94,10 +94,10 @@ func ResolveK8sPatchVersion(k8sVersion string) (string, error) { return extendedVersion, nil } -// K8sVersionFromMajorMinor takes a semver in format MAJOR.MINOR +// k8sVersionFromMajorMinor takes a semver in format MAJOR.MINOR // and returns the version in format MAJOR.MINOR.PATCH with the // supported patch version as PATCH. -func K8sVersionFromMajorMinor(version string) string { +func k8sVersionFromMajorMinor(version string) string { switch version { case semver.MajorMinor(string(V1_26)): return string(V1_26)