Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cli: store kubernetes version as strong type in config #2287

Merged
merged 14 commits into from
Sep 19, 2023
46 changes: 10 additions & 36 deletions cli/internal/cmd/configgenerate.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ 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"
Expand All @@ -35,15 +34,15 @@ 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", 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
}

type generateFlags struct {
pf pathprefix.PathPrefixer
k8sVersion string
k8sVersion versions.ValidK8sVersion
attestationVariant variant.Variant
}

Expand Down Expand Up @@ -124,31 +123,22 @@ 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 {
return generateFlags{}, fmt.Errorf("parsing workspace flag: %w", err)
}
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 := versions.ResolveK8sPatchVersion(k8sVersion)
if err != nil {
return generateFlags{}, fmt.Errorf("resolving kubernetes patch version from flag: %w", err)
}
resolvedVersion, err := resolveK8sVersion(k8sVersion)
validK8sVersion, err := versions.NewValidK8sVersion(resolvedVersion, true)
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")
Expand All @@ -168,7 +158,7 @@ func parseGenerateFlags(cmd *cobra.Command) (generateFlags, error) {
}
return generateFlags{
pf: pathprefix.New(workDir),
k8sVersion: resolvedVersion,
k8sVersion: validK8sVersion,
attestationVariant: attestationVariant,
}, nil
}
Expand All @@ -184,22 +174,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), "|"))
}
Expand Down
31 changes: 27 additions & 4 deletions cli/internal/cmd/configgenerate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package cmd

import (
"fmt"
"strings"
"testing"

"github.com/edgelesssys/constellation/v2/internal/attestation/variant"
Expand All @@ -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,
Expand All @@ -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)
Expand Down
16 changes: 7 additions & 9 deletions cli/internal/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -153,6 +152,11 @@ func (i *initCmd) initialize(
if err != nil {
return err
}
// cfg validation does not check k8s patch version since upgrade may accept an outdated patch version.
k8sVersion, err := versions.NewValidK8sVersion(string(conf.KubernetesVersion), true)
if err != nil {
return err
}
if !flags.force {
if err := validateCLIandConstellationVersionAreEqual(constants.BinaryVersion(), conf.Image, conf.MicroserviceVersion); err != nil {
return err
Expand All @@ -168,12 +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, err := versions.NewValidK8sVersion(compatibility.EnsurePrefixV(conf.KubernetesVersion), true)
if err != nil {
return err
}
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)
Expand Down Expand Up @@ -275,7 +273,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)
Expand Down Expand Up @@ -629,7 +627,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 {
Expand Down
27 changes: 22 additions & 5 deletions cli/internal/cmd/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -125,15 +125,32 @@ 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,
},
"k8s version without v works": {
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)
require.NoError(t, err)
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
},
}

Expand Down Expand Up @@ -222,7 +239,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
}

Expand Down
8 changes: 4 additions & 4 deletions cli/internal/cmd/upgradeapply.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl
}
}
}
validK8sVersion, err := validK8sVersion(cmd, conf.KubernetesVersion, flags.yes)
conf.KubernetesVersion, err = validK8sVersion(cmd, string(conf.KubernetesVersion), flags.yes)
elchead marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
Expand Down Expand Up @@ -223,7 +223,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)
Expand Down Expand Up @@ -400,7 +400,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 {
Expand All @@ -418,7 +418,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 {
Expand Down
34 changes: 31 additions & 3 deletions cli/internal/cmd/upgradeapply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -37,6 +38,7 @@ func TestUpgradeApply(t *testing.T) {
kubeUpgrader *stubKubernetesUpgrader
terraformUpgrader clusterUpgrader
wantErr bool
customK8sVersion string
flags upgradeApplyFlags
stdin string
}{
Expand Down Expand Up @@ -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()
}(),
flags: upgradeApplyFlags{yes: true},
wantErr: false,
},
"outdated K8s version": {
kubeUpgrader: &stubKubernetesUpgrader{
currentConfig: config.DefaultForAzureSEVSNP(),
},
helmUpgrader: stubApplier{},
terraformUpgrader: &stubTerraformUpgrader{},
customK8sVersion: "v1.20.0",
flags: upgradeApplyFlags{yes: true},
wantErr: true,
},
"skip all upgrade phases": {
kubeUpgrader: &stubKubernetesUpgrader{
currentConfig: config.DefaultForAzureSEVSNP(),
Expand Down Expand Up @@ -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{}))
Expand Down Expand Up @@ -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)
}
6 changes: 5 additions & 1 deletion cli/internal/cmd/upgradecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 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]
Expand Down
1 change: 0 additions & 1 deletion cli/internal/helm/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading