From b25228d175c96f4b969a78186ed5befb2c19b204 Mon Sep 17 00:00:00 2001 From: Moritz Sanft <58110325+msanft@users.noreply.github.com> Date: Wed, 21 Jun 2023 09:22:32 +0200 Subject: [PATCH] cli: store upgrade files in versioned folders (#1929) * upgrade versioning * dont pass upgrade kind as boolean * whitespace * fix godot lint check * clarify upgrade check directory suffix * cli: dry-run Terraform migrations on `upgrade check` (#1942) * dry-run Terraform migrations on upgrade check * clean whole upgrade dir * clean up check workspace after planning * fix parsing * extend upgrade check test * rename unused parameters * exclude false positives in test --- cli/internal/cmd/upgradeapply.go | 7 +- cli/internal/cmd/upgradecheck.go | 94 ++++++++++++++++---- cli/internal/cmd/upgradecheck_test.go | 75 +++++++++++----- cli/internal/helm/backup.go | 19 ++-- cli/internal/helm/backup_test.go | 14 ++- cli/internal/helm/client.go | 6 +- cli/internal/kubernetes/BUILD.bazel | 1 + cli/internal/kubernetes/upgrade.go | 57 ++++++++---- cli/internal/terraform/loader.go | 5 +- cli/internal/terraform/loader_test.go | 8 +- cli/internal/terraform/terraform.go | 4 +- cli/internal/upgrade/terraform.go | 22 ++--- cli/internal/upgrade/terraform_test.go | 115 +++++++++++++++++-------- 13 files changed, 300 insertions(+), 127 deletions(-) diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 7e15cc845..a8d895e65 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -62,7 +62,7 @@ func runUpgradeApply(cmd *cobra.Command, _ []string) error { defer log.Sync() fileHandler := file.NewHandler(afero.NewOsFs()) - upgrader, err := kubernetes.NewUpgrader(cmd.Context(), cmd.OutOrStdout(), log) + upgrader, err := kubernetes.NewUpgrader(cmd.Context(), cmd.OutOrStdout(), log, kubernetes.UpgradeCmdKindApply) if err != nil { return err } @@ -149,7 +149,7 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, file file.Handler return fmt.Errorf("checking workspace: %w", err) } - targets, vars, err := u.parseUpgradeVars(cmd, conf, fetcher) + targets, vars, err := parseTerraformUpgradeVars(cmd, conf, fetcher) if err != nil { return fmt.Errorf("parsing upgrade variables: %w", err) } @@ -201,7 +201,8 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, file file.Handler return nil } -func (u *upgradeApplyCmd) parseUpgradeVars(cmd *cobra.Command, conf *config.Config, fetcher imageFetcher) ([]string, terraform.Variables, error) { +// parseTerraformUpgradeVars parses the variables required to execute the Terraform script with. +func parseTerraformUpgradeVars(cmd *cobra.Command, conf *config.Config, fetcher imageFetcher) ([]string, terraform.Variables, error) { // Fetch variables to execute Terraform script with provider := conf.GetProvider() attestationVariant := conf.GetAttestationConfig().GetVariant() diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index 0a8ea1f6f..89fecf484 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -18,6 +18,8 @@ import ( "github.com/edgelesssys/constellation/v2/cli/internal/featureset" "github.com/edgelesssys/constellation/v2/cli/internal/helm" "github.com/edgelesssys/constellation/v2/cli/internal/kubernetes" + "github.com/edgelesssys/constellation/v2/cli/internal/terraform" + "github.com/edgelesssys/constellation/v2/cli/internal/upgrade" "github.com/edgelesssys/constellation/v2/internal/api/attestationconfigapi" "github.com/edgelesssys/constellation/v2/internal/api/fetcher" "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" @@ -28,6 +30,7 @@ import ( "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/imagefetcher" "github.com/edgelesssys/constellation/v2/internal/kubernetes/kubectl" conSemver "github.com/edgelesssys/constellation/v2/internal/semver" "github.com/edgelesssys/constellation/v2/internal/sigstore" @@ -65,7 +68,7 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { if err != nil { return err } - checker, err := kubernetes.NewUpgrader(cmd.Context(), cmd.OutOrStdout(), log) + checker, err := kubernetes.NewUpgrader(cmd.Context(), cmd.OutOrStdout(), log, kubernetes.UpgradeCmdKindCheck) if err != nil { return err } @@ -89,7 +92,9 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { log: log, versionsapi: versionfetcher, }, - log: log, + checker: checker, + imagefetcher: imagefetcher.New(), + log: log, } return up.upgradeCheck(cmd, fileHandler, attestationconfigapi.NewFetcher(), flags) @@ -98,36 +103,49 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { func parseUpgradeCheckFlags(cmd *cobra.Command) (upgradeCheckFlags, error) { configPath, err := cmd.Flags().GetString("config") if err != nil { - return upgradeCheckFlags{}, err + return upgradeCheckFlags{}, fmt.Errorf("parsing config string: %w", err) } force, err := cmd.Flags().GetBool("force") if err != nil { - return upgradeCheckFlags{}, err + return upgradeCheckFlags{}, fmt.Errorf("parsing force bool: %w", err) } writeConfig, err := cmd.Flags().GetBool("write-config") if err != nil { - return upgradeCheckFlags{}, err + return upgradeCheckFlags{}, fmt.Errorf("parsing write-config bool: %w", err) } ref, err := cmd.Flags().GetString("ref") if err != nil { - return upgradeCheckFlags{}, err + return upgradeCheckFlags{}, fmt.Errorf("parsing ref string: %w", err) } stream, err := cmd.Flags().GetString("stream") if err != nil { - return upgradeCheckFlags{}, err + return upgradeCheckFlags{}, fmt.Errorf("parsing stream string: %w", err) } + + logLevelString, err := cmd.Flags().GetString("tf-log") + if err != nil { + return upgradeCheckFlags{}, fmt.Errorf("parsing tf-log string: %w", err) + } + logLevel, err := terraform.ParseLogLevel(logLevelString) + if err != nil { + return upgradeCheckFlags{}, fmt.Errorf("parsing Terraform log level %s: %w", logLevelString, err) + } + return upgradeCheckFlags{ - configPath: configPath, - force: force, - writeConfig: writeConfig, - ref: ref, - stream: stream, + configPath: configPath, + force: force, + writeConfig: writeConfig, + ref: ref, + stream: stream, + terraformLogLevel: logLevel, }, nil } type upgradeCheckCmd struct { canUpgradeCheck bool collect collector + checker upgradeChecker + imagefetcher imageFetcher log debugLog } @@ -184,6 +202,44 @@ func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fileHandler file.Hand return err } + u.log.Debugf("Planning Terraform migrations") + + if err := u.checker.CheckTerraformMigrations(fileHandler); err != nil { + return fmt.Errorf("checking workspace: %w", err) + } + + targets, vars, err := parseTerraformUpgradeVars(cmd, conf, u.imagefetcher) + if err != nil { + return fmt.Errorf("parsing upgrade variables: %w", err) + } + u.log.Debugf("Using migration targets:\n%v", targets) + u.log.Debugf("Using Terraform variables:\n%v", vars) + + opts := upgrade.TerraformUpgradeOptions{ + LogLevel: flags.terraformLogLevel, + CSP: conf.GetProvider(), + Vars: vars, + Targets: targets, + OutputFile: constants.TerraformMigrationOutputFile, + } + + cmd.Println("The following Teraform migrations are available with this CLI:") + + // Check if there are any Terraform migrations + hasDiff, err := u.checker.PlanTerraformMigrations(cmd.Context(), opts) + if err != nil { + return fmt.Errorf("planning terraform migrations: %w", err) + } + defer func() { + if err := u.checker.CleanUpTerraformMigrations(fileHandler); err != nil { + u.log.Debugf("Failed to clean up Terraform migrations: %v", err) + } + }() + + if !hasDiff { + cmd.Println(" No Terraform migrations are available.") + } + upgrade := versionUpgrade{ newServices: newServices, newImages: newImages, @@ -661,16 +717,20 @@ func (v *versionCollector) filterCompatibleCLIVersions(ctx context.Context, cliP } type upgradeCheckFlags struct { - configPath string - force bool - writeConfig bool - ref string - stream string + configPath string + force bool + writeConfig bool + ref string + stream string + terraformLogLevel terraform.LogLevel } type upgradeChecker interface { CurrentImage(ctx context.Context) (string, error) CurrentKubernetesVersion(ctx context.Context) (string, error) + PlanTerraformMigrations(ctx context.Context, opts upgrade.TerraformUpgradeOptions) (bool, error) + CheckTerraformMigrations(fileHandler file.Handler) error + CleanUpTerraformMigrations(fileHandler file.Handler) error } type versionListFetcher interface { diff --git a/cli/internal/cmd/upgradecheck_test.go b/cli/internal/cmd/upgradecheck_test.go index 04402e788..f5507b097 100644 --- a/cli/internal/cmd/upgradecheck_test.go +++ b/cli/internal/cmd/upgradecheck_test.go @@ -15,6 +15,7 @@ import ( "strings" "testing" + "github.com/edgelesssys/constellation/v2/cli/internal/upgrade" "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" "github.com/edgelesssys/constellation/v2/internal/attestation/measurements" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" @@ -224,34 +225,53 @@ func TestUpgradeCheck(t *testing.T) { Version: "v2.5.0", Kind: versionsapi.VersionKindImage, } + collector := stubVersionCollector{ + supportedServicesVersions: "v2.5.0", + supportedImages: []versionsapi.Version{v2_3}, + supportedImageVersions: map[string]measurements.M{ + "v2.3.0": measurements.DefaultsFor(cloudprovider.GCP, variant.GCPSEVES{}), + }, + supportedK8sVersions: []string{"v1.24.5", "v1.24.12", "v1.25.6"}, + currentServicesVersions: "v2.4.0", + currentImageVersion: "v2.4.0", + currentK8sVersion: "v1.24.5", + currentCLIVersion: "v2.4.0", + images: []versionsapi.Version{v2_5}, + newCLIVersionsList: []string{"v2.5.0", "v2.6.0"}, + } + testCases := map[string]struct { - collector stubVersionCollector - flags upgradeCheckFlags - csp cloudprovider.Provider - cliVersion string - wantError bool + collector stubVersionCollector + flags upgradeCheckFlags + csp cloudprovider.Provider + checker stubUpgradeChecker + imagefetcher stubImageFetcher + cliVersion string + wantError bool }{ "upgrades gcp": { - collector: stubVersionCollector{ - supportedServicesVersions: "v2.5.0", - supportedImages: []versionsapi.Version{v2_3}, - supportedImageVersions: map[string]measurements.M{ - "v2.3.0": measurements.DefaultsFor(cloudprovider.GCP, variant.GCPSEVES{}), - }, - supportedK8sVersions: []string{"v1.24.5", "v1.24.12", "v1.25.6"}, - currentServicesVersions: "v2.4.0", - currentImageVersion: "v2.4.0", - currentK8sVersion: "v1.24.5", - currentCLIVersion: "v2.4.0", - images: []versionsapi.Version{v2_5}, - newCLIVersionsList: []string{"v2.5.0", "v2.6.0"}, - }, + collector: collector, + checker: stubUpgradeChecker{}, + imagefetcher: stubImageFetcher{}, flags: upgradeCheckFlags{ configPath: constants.ConfigFilename, }, csp: cloudprovider.GCP, cliVersion: "v1.0.0", }, + "terraform err": { + collector: collector, + checker: stubUpgradeChecker{ + err: assert.AnError, + }, + imagefetcher: stubImageFetcher{}, + flags: upgradeCheckFlags{ + configPath: constants.ConfigFilename, + }, + csp: cloudprovider.GCP, + cliVersion: "v1.0.0", + wantError: true, + }, } for name, tc := range testCases { @@ -266,6 +286,8 @@ func TestUpgradeCheck(t *testing.T) { checkCmd := upgradeCheckCmd{ canUpgradeCheck: true, collect: &tc.collector, + checker: tc.checker, + imagefetcher: tc.imagefetcher, log: logger.NewTest(t), } @@ -338,6 +360,7 @@ func (s *stubVersionCollector) filterCompatibleCLIVersions(_ context.Context, _ type stubUpgradeChecker struct { image string k8sVersion string + tfDiff bool err error } @@ -345,10 +368,22 @@ func (u stubUpgradeChecker) CurrentImage(context.Context) (string, error) { return u.image, u.err } -func (u stubUpgradeChecker) CurrentKubernetesVersion(_ context.Context) (string, error) { +func (u stubUpgradeChecker) CurrentKubernetesVersion(context.Context) (string, error) { return u.k8sVersion, u.err } +func (u stubUpgradeChecker) PlanTerraformMigrations(context.Context, upgrade.TerraformUpgradeOptions) (bool, error) { + return u.tfDiff, u.err +} + +func (u stubUpgradeChecker) CheckTerraformMigrations(file.Handler) error { + return u.err +} + +func (u stubUpgradeChecker) CleanUpTerraformMigrations(file.Handler) error { + return u.err +} + func TestNewCLIVersions(t *testing.T) { someErr := errors.New("some error") minorList := func() versionsapi.List { diff --git a/cli/internal/helm/backup.go b/cli/internal/helm/backup.go index f0da75bf2..d7ec77a4a 100644 --- a/cli/internal/helm/backup.go +++ b/cli/internal/helm/backup.go @@ -17,17 +17,13 @@ import ( "sigs.k8s.io/yaml" ) -var ( - backupFolder = filepath.Join(constants.UpgradeDir, "backups") + string(filepath.Separator) - crdBackupFolder = filepath.Join(backupFolder, "crds") + string(filepath.Separator) -) - -func (c *Client) backupCRDs(ctx context.Context) ([]apiextensionsv1.CustomResourceDefinition, error) { +func (c *Client) backupCRDs(ctx context.Context, upgradeID string) ([]apiextensionsv1.CustomResourceDefinition, error) { crds, err := c.kubectl.GetCRDs(ctx) if err != nil { return nil, fmt.Errorf("getting CRDs: %w", err) } + crdBackupFolder := c.crdBackupFolder(upgradeID) if err := c.fs.MkdirAll(crdBackupFolder); err != nil { return nil, fmt.Errorf("creating backup dir: %w", err) } @@ -54,7 +50,7 @@ func (c *Client) backupCRDs(ctx context.Context) ([]apiextensionsv1.CustomResour return crds, nil } -func (c *Client) backupCRs(ctx context.Context, crds []apiextensionsv1.CustomResourceDefinition) error { +func (c *Client) backupCRs(ctx context.Context, crds []apiextensionsv1.CustomResourceDefinition, upgradeID string) error { for _, crd := range crds { for _, version := range crd.Spec.Versions { gvr := schema.GroupVersionResource{Group: crd.Spec.Group, Version: version.Name, Resource: crd.Spec.Names.Plural} @@ -63,6 +59,7 @@ func (c *Client) backupCRs(ctx context.Context, crds []apiextensionsv1.CustomRes return fmt.Errorf("retrieving CR %s: %w", crd.Name, err) } + backupFolder := c.backupFolder(upgradeID) for _, cr := range crs { targetFolder := filepath.Join(backupFolder, gvr.Group, gvr.Version, cr.GetNamespace(), cr.GetKind()) if err := c.fs.MkdirAll(targetFolder); err != nil { @@ -83,3 +80,11 @@ func (c *Client) backupCRs(ctx context.Context, crds []apiextensionsv1.CustomRes } return nil } + +func (c *Client) backupFolder(upgradeID string) string { + return filepath.Join(constants.UpgradeDir, upgradeID, "backups") + string(filepath.Separator) +} + +func (c *Client) crdBackupFolder(upgradeID string) string { + return filepath.Join(c.backupFolder(upgradeID), "crds") + string(filepath.Separator) +} diff --git a/cli/internal/helm/backup_test.go b/cli/internal/helm/backup_test.go index 5ae71e929..ea87015e7 100644 --- a/cli/internal/helm/backup_test.go +++ b/cli/internal/helm/backup_test.go @@ -24,16 +24,19 @@ import ( func TestBackupCRDs(t *testing.T) { testCases := map[string]struct { + upgradeID string crd string expectedFile string getCRDsError error wantError bool }{ "success": { + upgradeID: "1234", crd: "apiVersion: \nkind: \nmetadata:\n name: foobar\n creationTimestamp: null\nspec:\n group: \"\"\n names:\n kind: \"somename\"\n plural: \"somenames\"\n scope: \"\"\n versions: null\nstatus:\n acceptedNames:\n kind: \"\"\n plural: \"\"\n conditions: null\n storedVersions: null\n", expectedFile: "apiVersion: apiextensions.k8s.io/v1\nkind: CustomResourceDefinition\nmetadata:\n name: foobar\n creationTimestamp: null\nspec:\n group: \"\"\n names:\n kind: \"somename\"\n plural: \"somenames\"\n scope: \"\"\n versions: null\nstatus:\n acceptedNames:\n kind: \"\"\n plural: \"\"\n conditions: null\n storedVersions: null\n", }, "api request fails": { + upgradeID: "1234", getCRDsError: errors.New("api error"), wantError: true, }, @@ -55,14 +58,14 @@ func TestBackupCRDs(t *testing.T) { log: stubLog{}, } - _, err = client.backupCRDs(context.Background()) + _, err = client.backupCRDs(context.Background(), tc.upgradeID) if tc.wantError { assert.Error(err) return } assert.NoError(err) - data, err := afero.ReadFile(memFs, filepath.Join(crdBackupFolder, crd.Name+".yaml")) + data, err := afero.ReadFile(memFs, filepath.Join(client.crdBackupFolder(tc.upgradeID), crd.Name+".yaml")) require.NoError(err) assert.YAMLEq(tc.expectedFile, string(data)) }) @@ -71,6 +74,7 @@ func TestBackupCRDs(t *testing.T) { func TestBackupCRs(t *testing.T) { testCases := map[string]struct { + upgradeID string crd apiextensionsv1.CustomResourceDefinition resource unstructured.Unstructured expectedFile string @@ -78,6 +82,7 @@ func TestBackupCRs(t *testing.T) { wantError bool }{ "success": { + upgradeID: "1234", crd: apiextensionsv1.CustomResourceDefinition{ Spec: apiextensionsv1.CustomResourceDefinitionSpec{ Names: apiextensionsv1.CustomResourceDefinitionNames{ @@ -95,6 +100,7 @@ func TestBackupCRs(t *testing.T) { expectedFile: "metadata:\n name: foobar\n", }, "api request fails": { + upgradeID: "1234", crd: apiextensionsv1.CustomResourceDefinition{ Spec: apiextensionsv1.CustomResourceDefinitionSpec{ Names: apiextensionsv1.CustomResourceDefinitionNames{ @@ -126,14 +132,14 @@ func TestBackupCRs(t *testing.T) { log: stubLog{}, } - err := client.backupCRs(context.Background(), []apiextensionsv1.CustomResourceDefinition{tc.crd}) + err := client.backupCRs(context.Background(), []apiextensionsv1.CustomResourceDefinition{tc.crd}, tc.upgradeID) if tc.wantError { assert.Error(err) return } assert.NoError(err) - data, err := afero.ReadFile(memFs, filepath.Join(backupFolder, tc.crd.Spec.Group, tc.crd.Spec.Versions[0].Name, tc.resource.GetNamespace(), tc.resource.GetKind(), tc.resource.GetName()+".yaml")) + data, err := afero.ReadFile(memFs, filepath.Join(client.backupFolder(tc.upgradeID), tc.crd.Spec.Group, tc.crd.Spec.Versions[0].Name, tc.resource.GetNamespace(), tc.resource.GetKind(), tc.resource.GetName()+".yaml")) require.NoError(err) assert.YAMLEq(tc.expectedFile, string(data)) }) diff --git a/cli/internal/helm/client.go b/cli/internal/helm/client.go index 9ab693ddc..ccccce3bf 100644 --- a/cli/internal/helm/client.go +++ b/cli/internal/helm/client.go @@ -102,7 +102,7 @@ func (c *Client) shouldUpgrade(releaseName, newVersion string) error { // Upgrade runs a helm-upgrade on all deployments that are managed via Helm. // If the CLI receives an interrupt signal it will cancel the context. // Canceling the context will prompt helm to abort and roll back the ongoing upgrade. -func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool) error { +func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool, upgradeID string) error { upgradeErrs := []error{} upgradeReleases := []*chart.Chart{} invalidUpgrade := &compatibility.InvalidUpgradeError{} @@ -138,11 +138,11 @@ func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout tim return errors.Join(upgradeErrs...) } - crds, err := c.backupCRDs(ctx) + crds, err := c.backupCRDs(ctx, upgradeID) if err != nil { return fmt.Errorf("creating CRD backup: %w", err) } - if err := c.backupCRs(ctx, crds); err != nil { + if err := c.backupCRs(ctx, crds, upgradeID); err != nil { return fmt.Errorf("creating CR backup: %w", err) } diff --git a/cli/internal/kubernetes/BUILD.bazel b/cli/internal/kubernetes/BUILD.bazel index b63851de5..d7b1d4d44 100644 --- a/cli/internal/kubernetes/BUILD.bazel +++ b/cli/internal/kubernetes/BUILD.bazel @@ -28,6 +28,7 @@ go_library( "//internal/versions", "//internal/versions/components", "//operators/constellation-node-operator/api/v1alpha1", + "@com_github_google_uuid//:uuid", "@io_k8s_api//core/v1:core", "@io_k8s_apimachinery//pkg/api/errors", "@io_k8s_apimachinery//pkg/apis/meta/v1:meta", diff --git a/cli/internal/kubernetes/upgrade.go b/cli/internal/kubernetes/upgrade.go index 9c3db1416..a36ac6681 100644 --- a/cli/internal/kubernetes/upgrade.go +++ b/cli/internal/kubernetes/upgrade.go @@ -33,6 +33,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/edgelesssys/constellation/v2/internal/versions/components" updatev1alpha1 "github.com/edgelesssys/constellation/v2/operators/constellation-node-operator/v2/api/v1alpha1" + "github.com/google/uuid" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,6 +45,16 @@ import ( "k8s.io/client-go/tools/clientcmd" ) +// UpgradeCmdKind is the kind of the upgrade command (check, apply). +type UpgradeCmdKind int + +const ( + // UpgradeCmdKindCheck corresponds to the upgrade check command. + UpgradeCmdKindCheck UpgradeCmdKind = iota + // UpgradeCmdKindApply corresponds to the upgrade apply command. + UpgradeCmdKindApply +) + // ErrInProgress signals that an upgrade is in progress inside the cluster. var ErrInProgress = errors.New("upgrade in progress") @@ -85,10 +96,26 @@ type Upgrader struct { outWriter io.Writer tfUpgrader *upgrade.TerraformUpgrader log debugLog + upgradeID string } // NewUpgrader returns a new Upgrader. -func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog) (*Upgrader, error) { +func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog, upgradeCmdKind UpgradeCmdKind) (*Upgrader, error) { + upgradeID := "upgrade-" + time.Now().Format("20060102150405") + "-" + strings.Split(uuid.New().String(), "-")[0] + if upgradeCmdKind == UpgradeCmdKindCheck { + // When performing an upgrade check, the upgrade directory will only be used temporarily to store the + // Terraform state. The directory is deleted after the check is finished. + // Therefore, add a tmp-suffix to the upgrade ID to indicate that the directory will be cleared after the check. + upgradeID += "-tmp" + } + + u := &Upgrader{ + imageFetcher: imagefetcher.New(), + outWriter: outWriter, + log: log, + upgradeID: upgradeID, + } + kubeConfig, err := clientcmd.BuildConfigFromFlags("", constants.AdminConfFilename) if err != nil { return nil, fmt.Errorf("building kubernetes config: %w", err) @@ -98,19 +125,22 @@ func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog) (*Upgra if err != nil { return nil, fmt.Errorf("setting up kubernetes client: %w", err) } + u.stableInterface = &stableClient{client: kubeClient} // use unstructured client to avoid importing the operator packages unstructuredClient, err := dynamic.NewForConfig(kubeConfig) if err != nil { return nil, fmt.Errorf("setting up custom resource client: %w", err) } + u.dynamicInterface = &NodeVersionClient{client: unstructuredClient} helmClient, err := helm.NewClient(kubectl.New(), constants.AdminConfFilename, constants.HelmNamespace, log) if err != nil { return nil, fmt.Errorf("setting up helm client: %w", err) } + u.helmClient = helmClient - tfClient, err := terraform.New(ctx, filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir)) + tfClient, err := terraform.New(ctx, filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeWorkingDir)) if err != nil { return nil, fmt.Errorf("setting up terraform client: %w", err) } @@ -119,35 +149,28 @@ func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog) (*Upgra if err != nil { return nil, fmt.Errorf("setting up terraform upgrader: %w", err) } + u.tfUpgrader = tfUpgrader - return &Upgrader{ - stableInterface: &stableClient{client: kubeClient}, - dynamicInterface: &NodeVersionClient{client: unstructuredClient}, - helmClient: helmClient, - imageFetcher: imagefetcher.New(), - outWriter: outWriter, - tfUpgrader: tfUpgrader, - log: log, - }, nil + return u, nil } // CheckTerraformMigrations checks whether Terraform migrations are possible in the current workspace. // If the files that will be written during the upgrade already exist, it returns an error. func (u *Upgrader) CheckTerraformMigrations(fileHandler file.Handler) error { - return u.tfUpgrader.CheckTerraformMigrations(fileHandler) + return u.tfUpgrader.CheckTerraformMigrations(fileHandler, u.upgradeID) } // CleanUpTerraformMigrations cleans up the Terraform migration workspace, for example when an upgrade is // aborted by the user. func (u *Upgrader) CleanUpTerraformMigrations(fileHandler file.Handler) error { - return u.tfUpgrader.CleanUpTerraformMigrations(fileHandler) + return u.tfUpgrader.CleanUpTerraformMigrations(fileHandler, u.upgradeID) } // PlanTerraformMigrations prepares the upgrade workspace and plans the Terraform migrations for the Constellation upgrade. // If a diff exists, it's being written to the upgrader's output writer. It also returns // a bool indicating whether a diff exists. func (u *Upgrader) PlanTerraformMigrations(ctx context.Context, opts upgrade.TerraformUpgradeOptions) (bool, error) { - return u.tfUpgrader.PlanTerraformMigrations(ctx, opts) + return u.tfUpgrader.PlanTerraformMigrations(ctx, opts, u.upgradeID) } // ApplyTerraformMigrations applies the migerations planned by PlanTerraformMigrations. @@ -155,12 +178,12 @@ func (u *Upgrader) PlanTerraformMigrations(ctx context.Context, opts upgrade.Ter // In case of a successful upgrade, the output will be written to the specified file and the old Terraform directory is replaced // By the new one. func (u *Upgrader) ApplyTerraformMigrations(ctx context.Context, fileHandler file.Handler, opts upgrade.TerraformUpgradeOptions) error { - return u.tfUpgrader.ApplyTerraformMigrations(ctx, fileHandler, opts) + return u.tfUpgrader.ApplyTerraformMigrations(ctx, fileHandler, opts, u.upgradeID) } // UpgradeHelmServices upgrade helm services. func (u *Upgrader) UpgradeHelmServices(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool) error { - return u.helmClient.Upgrade(ctx, config, timeout, allowDestructive) + return u.helmClient.Upgrade(ctx, config, timeout, allowDestructive, u.upgradeID) } // UpgradeNodeVersion upgrades the cluster's NodeVersion object and in turn triggers image & k8s version upgrades. @@ -526,7 +549,7 @@ func joinConfigMigration(existingConf *corev1.ConfigMap, attestVariant variant.V } type helmInterface interface { - Upgrade(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool) error + Upgrade(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool, upgradeID string) error } type debugLog interface { diff --git a/cli/internal/terraform/loader.go b/cli/internal/terraform/loader.go index 94dcf3aca..2c1bc7f52 100644 --- a/cli/internal/terraform/loader.go +++ b/cli/internal/terraform/loader.go @@ -16,7 +16,6 @@ import ( "path/filepath" "strings" - "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/spf13/afero" ) @@ -36,11 +35,11 @@ func prepareWorkspace(rootDir string, fileHandler file.Handler, workingDir strin // prepareUpgradeWorkspace takes the Terraform state file from the old workspace and the // embedded Terraform files and writes them into the new workspace. -func prepareUpgradeWorkspace(rootDir string, fileHandler file.Handler, oldWorkingDir, newWorkingDir string) error { +func prepareUpgradeWorkspace(rootDir string, fileHandler file.Handler, oldWorkingDir, newWorkingDir, backupDir string) error { // backup old workspace if err := fileHandler.CopyDir( oldWorkingDir, - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), + backupDir, ); err != nil { return fmt.Errorf("backing up old workspace: %w", err) } diff --git a/cli/internal/terraform/loader_test.go b/cli/internal/terraform/loader_test.go index fa1b3a755..0f5cf0d3f 100644 --- a/cli/internal/terraform/loader_test.go +++ b/cli/internal/terraform/loader_test.go @@ -133,6 +133,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { provider cloudprovider.Provider oldWorkingDir string newWorkingDir string + backupDir string oldWorkspaceFiles []string newWorkspaceFiles []string expectedFiles []string @@ -144,6 +145,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { provider: cloudprovider.AWS, oldWorkingDir: "old", newWorkingDir: "new", + backupDir: "backup", oldWorkspaceFiles: []string{"terraform.tfstate"}, expectedFiles: []string{ "main.tf", @@ -158,6 +160,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { provider: cloudprovider.AWS, oldWorkingDir: "old", newWorkingDir: "new", + backupDir: "backup", oldWorkspaceFiles: []string{}, expectedFiles: []string{}, wantErr: true, @@ -167,6 +170,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { provider: cloudprovider.AWS, oldWorkingDir: "old", newWorkingDir: "new", + backupDir: "backup", oldWorkspaceFiles: []string{"terraform.tfstate"}, newWorkspaceFiles: []string{"main.tf"}, wantErr: true, @@ -185,7 +189,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { createFiles(t, file, tc.oldWorkspaceFiles, tc.oldWorkingDir) createFiles(t, file, tc.newWorkspaceFiles, tc.newWorkingDir) - err := prepareUpgradeWorkspace(path, file, tc.oldWorkingDir, tc.newWorkingDir) + err := prepareUpgradeWorkspace(path, file, tc.oldWorkingDir, tc.newWorkingDir, tc.backupDir) if tc.wantErr { require.Error(err) @@ -194,7 +198,7 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { } checkFiles(t, file, func(err error) { assert.NoError(err) }, tc.newWorkingDir, tc.expectedFiles) checkFiles(t, file, func(err error) { assert.NoError(err) }, - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), + tc.backupDir, tc.oldWorkspaceFiles, ) }) diff --git a/cli/internal/terraform/terraform.go b/cli/internal/terraform/terraform.go index 691d9e5a3..2adf166c8 100644 --- a/cli/internal/terraform/terraform.go +++ b/cli/internal/terraform/terraform.go @@ -87,8 +87,8 @@ func (c *Client) PrepareWorkspace(path string, vars Variables) error { // PrepareUpgradeWorkspace prepares a Terraform workspace for a Constellation version upgrade. // It copies the Terraform state from the old working dir and the embedded Terraform files into the new working dir. -func (c *Client) PrepareUpgradeWorkspace(path, oldWorkingDir, newWorkingDir string, vars Variables) error { - if err := prepareUpgradeWorkspace(path, c.file, oldWorkingDir, newWorkingDir); err != nil { +func (c *Client) PrepareUpgradeWorkspace(path, oldWorkingDir, newWorkingDir, backupDir string, vars Variables) error { + if err := prepareUpgradeWorkspace(path, c.file, oldWorkingDir, newWorkingDir, backupDir); err != nil { return fmt.Errorf("prepare upgrade workspace: %w", err) } diff --git a/cli/internal/upgrade/terraform.go b/cli/internal/upgrade/terraform.go index 7e764de52..75cd0301f 100644 --- a/cli/internal/upgrade/terraform.go +++ b/cli/internal/upgrade/terraform.go @@ -54,11 +54,11 @@ type TerraformUpgradeOptions struct { // CheckTerraformMigrations checks whether Terraform migrations are possible in the current workspace. // If the files that will be written during the upgrade already exist, it returns an error. -func (u *TerraformUpgrader) CheckTerraformMigrations(fileHandler file.Handler) error { +func (u *TerraformUpgrader) CheckTerraformMigrations(fileHandler file.Handler, upgradeID string) error { var existingFiles []string filesToCheck := []string{ constants.TerraformMigrationOutputFile, - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), + filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeBackupDir), } for _, f := range filesToCheck { @@ -90,11 +90,12 @@ func checkFileExists(fileHandler file.Handler, existingFiles *[]string, filename // PlanTerraformMigrations prepares the upgrade workspace and plans the Terraform migrations for the Constellation upgrade. // If a diff exists, it's being written to the upgrader's output writer. It also returns // a bool indicating whether a diff exists. -func (u *TerraformUpgrader) PlanTerraformMigrations(ctx context.Context, opts TerraformUpgradeOptions) (bool, error) { +func (u *TerraformUpgrader) PlanTerraformMigrations(ctx context.Context, opts TerraformUpgradeOptions, upgradeID string) (bool, error) { err := u.tf.PrepareUpgradeWorkspace( filepath.Join("terraform", strings.ToLower(opts.CSP.String())), constants.TerraformWorkingDir, - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir), + filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeWorkingDir), + filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeBackupDir), opts.Vars, ) if err != nil { @@ -117,10 +118,9 @@ func (u *TerraformUpgrader) PlanTerraformMigrations(ctx context.Context, opts Te // CleanUpTerraformMigrations cleans up the Terraform migration workspace, for example when an upgrade is // aborted by the user. -func (u *TerraformUpgrader) CleanUpTerraformMigrations(fileHandler file.Handler) error { +func (u *TerraformUpgrader) CleanUpTerraformMigrations(fileHandler file.Handler, upgradeID string) error { cleanupFiles := []string{ - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir), + filepath.Join(constants.UpgradeDir, upgradeID), } for _, f := range cleanupFiles { @@ -136,7 +136,7 @@ func (u *TerraformUpgrader) CleanUpTerraformMigrations(fileHandler file.Handler) // If PlanTerraformMigrations has not been executed before, it will return an error. // In case of a successful upgrade, the output will be written to the specified file and the old Terraform directory is replaced // By the new one. -func (u *TerraformUpgrader) ApplyTerraformMigrations(ctx context.Context, fileHandler file.Handler, opts TerraformUpgradeOptions) error { +func (u *TerraformUpgrader) ApplyTerraformMigrations(ctx context.Context, fileHandler file.Handler, opts TerraformUpgradeOptions, upgradeID string) error { tfOutput, err := u.tf.CreateCluster(ctx, opts.LogLevel, opts.Targets...) if err != nil { return fmt.Errorf("terraform apply: %w", err) @@ -161,11 +161,11 @@ func (u *TerraformUpgrader) ApplyTerraformMigrations(ctx context.Context, fileHa return fmt.Errorf("removing old terraform directory: %w", err) } - if err := fileHandler.CopyDir(filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir), constants.TerraformWorkingDir); err != nil { + if err := fileHandler.CopyDir(filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeWorkingDir), constants.TerraformWorkingDir); err != nil { return fmt.Errorf("replacing old terraform directory with new one: %w", err) } - if err := fileHandler.RemoveAll(filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir)); err != nil { + if err := fileHandler.RemoveAll(filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeWorkingDir)); err != nil { return fmt.Errorf("removing terraform upgrade directory: %w", err) } @@ -178,7 +178,7 @@ func (u *TerraformUpgrader) ApplyTerraformMigrations(ctx context.Context, fileHa // a tfClient performs the Terraform interactions in an upgrade. type tfClient interface { - PrepareUpgradeWorkspace(path, oldWorkingDir, newWorkingDir string, vars terraform.Variables) error + PrepareUpgradeWorkspace(path, oldWorkingDir, newWorkingDir, upgradeID string, vars terraform.Variables) error ShowPlan(ctx context.Context, logLevel terraform.LogLevel, planFilePath string, output io.Writer) error Plan(ctx context.Context, logLevel terraform.LogLevel, planFile string, targets ...string) (bool, error) CreateCluster(ctx context.Context, logLevel terraform.LogLevel, targets ...string) (terraform.CreateOutput, error) diff --git a/cli/internal/upgrade/terraform_test.go b/cli/internal/upgrade/terraform_test.go index 091f8c806..82d0e93e8 100644 --- a/cli/internal/upgrade/terraform_test.go +++ b/cli/internal/upgrade/terraform_test.go @@ -40,18 +40,22 @@ func TestCheckTerraformMigrations(t *testing.T) { } testCases := map[string]struct { + upgradeID string workspace file.Handler wantErr bool }{ "success": { + upgradeID: "1234", workspace: workspace(nil), }, "migration output file already exists": { + upgradeID: "1234", workspace: workspace([]string{constants.TerraformMigrationOutputFile}), wantErr: true, }, "terraform backup dir already exists": { - workspace: workspace([]string{filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir)}), + upgradeID: "1234", + workspace: workspace([]string{filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeBackupDir)}), wantErr: true, }, } @@ -59,7 +63,7 @@ func TestCheckTerraformMigrations(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { u := upgrader() - err := u.CheckTerraformMigrations(tc.workspace) + err := u.CheckTerraformMigrations(tc.workspace, tc.upgradeID) if tc.wantErr { require.Error(t, err) return @@ -79,20 +83,24 @@ func TestPlanTerraformMigrations(t *testing.T) { } testCases := map[string]struct { - tf tfClient - want bool - wantErr bool + upgradeID string + tf tfClient + want bool + wantErr bool }{ "success no diff": { - tf: &stubTerraformClient{}, + upgradeID: "1234", + tf: &stubTerraformClient{}, }, "success diff": { + upgradeID: "1234", tf: &stubTerraformClient{ hasDiff: true, }, want: true, }, "prepare workspace error": { + upgradeID: "1234", tf: &stubTerraformClient{ prepareWorkspaceErr: assert.AnError, }, @@ -105,11 +113,13 @@ func TestPlanTerraformMigrations(t *testing.T) { wantErr: true, }, "show plan error no diff": { + upgradeID: "1234", tf: &stubTerraformClient{ showErr: assert.AnError, }, }, "show plan error diff": { + upgradeID: "1234", tf: &stubTerraformClient{ showErr: assert.AnError, hasDiff: true, @@ -130,7 +140,7 @@ func TestPlanTerraformMigrations(t *testing.T) { Vars: &terraform.QEMUVariables{}, } - diff, err := u.PlanTerraformMigrations(context.Background(), opts) + diff, err := u.PlanTerraformMigrations(context.Background(), opts, tc.upgradeID) if tc.wantErr { require.Error(err) } else { @@ -149,11 +159,11 @@ func TestApplyTerraformMigrations(t *testing.T) { return u } - fileHandler := func(existingFiles ...string) file.Handler { + fileHandler := func(upgradeID string, existingFiles ...string) file.Handler { fh := file.NewHandler(afero.NewMemMapFs()) require.NoError(t, fh.Write( - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir, "someFile"), + filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformUpgradeWorkingDir, "someFile"), []byte("some content"), )) for _, f := range existingFiles { @@ -163,6 +173,7 @@ func TestApplyTerraformMigrations(t *testing.T) { } testCases := map[string]struct { + upgradeID string tf tfClient policyPatcher stubPolicyPatcher fs file.Handler @@ -170,38 +181,43 @@ func TestApplyTerraformMigrations(t *testing.T) { wantErr bool }{ "success": { + upgradeID: "1234", tf: &stubTerraformClient{}, - fs: fileHandler(), + fs: fileHandler("1234"), policyPatcher: stubPolicyPatcher{}, outputFileName: "test.json", }, "create cluster error": { + upgradeID: "1234", tf: &stubTerraformClient{ CreateClusterErr: assert.AnError, }, - fs: fileHandler(), + fs: fileHandler("1234"), policyPatcher: stubPolicyPatcher{}, outputFileName: "test.json", wantErr: true, }, "patch error": { - tf: &stubTerraformClient{}, - fs: fileHandler(), + upgradeID: "1234", + tf: &stubTerraformClient{}, + fs: fileHandler("1234"), policyPatcher: stubPolicyPatcher{ patchErr: assert.AnError, }, wantErr: true, }, "empty file name": { + upgradeID: "1234", tf: &stubTerraformClient{}, - fs: fileHandler(), + fs: fileHandler("1234"), policyPatcher: stubPolicyPatcher{}, outputFileName: "", wantErr: true, }, "file already exists": { + upgradeID: "1234", tf: &stubTerraformClient{}, - fs: fileHandler("test.json"), + fs: fileHandler("1234", "test.json"), policyPatcher: stubPolicyPatcher{}, outputFileName: "test.json", wantErr: true, @@ -221,7 +237,7 @@ func TestApplyTerraformMigrations(t *testing.T) { OutputFile: tc.outputFileName, } - err := u.ApplyTerraformMigrations(context.Background(), tc.fs, opts) + err := u.ApplyTerraformMigrations(context.Background(), tc.fs, opts, tc.upgradeID) if tc.wantErr { require.Error(err) } else { @@ -249,33 +265,47 @@ func TestCleanUpTerraformMigrations(t *testing.T) { } testCases := map[string]struct { - workspace file.Handler - wantFiles []string - wantErr bool + upgradeID string + workspaceFiles []string + wantFiles []string + wantErr bool }{ "no files": { - workspace: workspace(nil), - wantFiles: []string{}, + upgradeID: "1234", + workspaceFiles: nil, + wantFiles: []string{}, }, "clean backup dir": { - workspace: workspace([]string{ - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), - }), + upgradeID: "1234", + workspaceFiles: []string{ + filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeBackupDir), + }, wantFiles: []string{}, }, "clean working dir": { - workspace: workspace([]string{ - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeWorkingDir), - }), + upgradeID: "1234", + workspaceFiles: []string{ + filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeWorkingDir), + }, wantFiles: []string{}, }, - "clean backup dir leave other files": { - workspace: workspace([]string{ - filepath.Join(constants.UpgradeDir, constants.TerraformUpgradeBackupDir), - filepath.Join(constants.UpgradeDir, "someFile"), - }), + "clean all": { + upgradeID: "1234", + workspaceFiles: []string{ + filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeBackupDir), + filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeWorkingDir), + filepath.Join(constants.UpgradeDir, "1234", "abc"), + }, + wantFiles: []string{}, + }, + "leave other files": { + upgradeID: "1234", + workspaceFiles: []string{ + filepath.Join(constants.UpgradeDir, "1234", constants.TerraformUpgradeBackupDir), + filepath.Join(constants.UpgradeDir, "other"), + }, wantFiles: []string{ - filepath.Join(constants.UpgradeDir, "someFile"), + filepath.Join(constants.UpgradeDir, "other"), }, }, } @@ -284,8 +314,10 @@ func TestCleanUpTerraformMigrations(t *testing.T) { t.Run(name, func(t *testing.T) { require := require.New(t) + workspace := workspace(tc.workspaceFiles) u := upgrader() - err := u.CleanUpTerraformMigrations(tc.workspace) + + err := u.CleanUpTerraformMigrations(workspace, tc.upgradeID) if tc.wantErr { require.Error(err) return @@ -293,9 +325,16 @@ func TestCleanUpTerraformMigrations(t *testing.T) { require.NoError(err) - for _, f := range tc.wantFiles { - _, err := tc.workspace.Stat(f) - require.NoError(err, "file %s should exist", f) + for _, haveFile := range tc.workspaceFiles { + for _, wantFile := range tc.wantFiles { + if haveFile == wantFile { + _, err := workspace.Stat(wantFile) + require.NoError(err, "file %s should exist", wantFile) + } else { + _, err := workspace.Stat(haveFile) + require.Error(err, "file %s should not exist", haveFile) + } + } } }) } @@ -309,7 +348,7 @@ type stubTerraformClient struct { CreateClusterErr error } -func (u *stubTerraformClient) PrepareUpgradeWorkspace(string, string, string, terraform.Variables) error { +func (u *stubTerraformClient) PrepareUpgradeWorkspace(string, string, string, string, terraform.Variables) error { return u.prepareWorkspaceErr }