From 3edc1c3ebb5d2922682f78df60683430a49e2409 Mon Sep 17 00:00:00 2001 From: Malte Poll Date: Tue, 27 Jun 2023 13:12:50 +0200 Subject: [PATCH] cli: manual AWS terraform state transitions This commit is designed to be reverted in the future (AB#3248). Terraform does not implement moved blocks with dynamic targets: https://github.com/hashicorp/terraform/issues/31335 so we have to migrate the terraform state ourselves. --- cli/internal/cmd/BUILD.bazel | 1 + cli/internal/cmd/manualtfstatemigration.go | 78 ++++++++++++++++++++++ cli/internal/cmd/upgradeapply.go | 12 ++++ cli/internal/cmd/upgradeapply_test.go | 7 ++ cli/internal/cmd/upgradecheck.go | 12 ++++ cli/internal/cmd/upgradecheck_test.go | 7 ++ cli/internal/kubernetes/upgrade.go | 15 ++++- cli/internal/terraform/terraform.go | 52 ++++++++++++++- cli/internal/terraform/terraform_test.go | 5 ++ 9 files changed, 183 insertions(+), 6 deletions(-) create mode 100644 cli/internal/cmd/manualtfstatemigration.go diff --git a/cli/internal/cmd/BUILD.bazel b/cli/internal/cmd/BUILD.bazel index da0a8188e..b13273366 100644 --- a/cli/internal/cmd/BUILD.bazel +++ b/cli/internal/cmd/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "iamdestroy.go", "init.go", "log.go", + "manualtfstatemigration.go", "mini.go", "minidown.go", "miniup.go", diff --git a/cli/internal/cmd/manualtfstatemigration.go b/cli/internal/cmd/manualtfstatemigration.go new file mode 100644 index 000000000..c0294ff06 --- /dev/null +++ b/cli/internal/cmd/manualtfstatemigration.go @@ -0,0 +1,78 @@ +/* +Copyright (c) Edgeless Systems GmbH + +SPDX-License-Identifier: AGPL-3.0-only +*/ + +package cmd + +import ( + "context" + "fmt" + + "github.com/edgelesssys/constellation/v2/cli/internal/terraform" + "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" +) + +// terraformMigrationAWSNodeGroups migrates the AWS node groups from the old state to the new state. +// TODO(AB#3248): Remove this migration after we can assume that all existing clusters have been migrated. +func terraformMigrationAWSNodeGroups(csp cloudprovider.Provider, zone string) []terraform.StateMigration { + if csp != cloudprovider.AWS { + return nil + } + return []terraform.StateMigration{ + { + DisplayName: "AWS node groups", + Hook: func(ctx context.Context, tfClient terraform.TFMigrator) error { + fromTo := []struct { + from string + to string + }{ + { + from: "aws_eip.lb", + to: fmt.Sprintf("aws_eip.lb[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_eip.nat", + to: fmt.Sprintf("module.public_private_subnet.aws_eip.nat[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_nat_gateway.gw", + to: fmt.Sprintf("module.public_private_subnet.aws_nat_gateway.gw[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_route_table.private_nat", + to: fmt.Sprintf("module.public_private_subnet.aws_route_table.private_nat[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_route_table.public_igw", + to: fmt.Sprintf("module.public_private_subnet.aws_route_table.public_igw[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_route_table_association.private-nat", + to: fmt.Sprintf("module.public_private_subnet.aws_route_table_association.private_nat[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_route_table_association.route_to_internet", + to: fmt.Sprintf("module.public_private_subnet.aws_route_table_association.route_to_internet[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_subnet.private", + to: fmt.Sprintf("module.public_private_subnet.aws_subnet.private[%q]", zone), + }, + { + from: "module.public_private_subnet.aws_subnet.public", + to: fmt.Sprintf("module.public_private_subnet.aws_subnet.public[%q]", zone), + }, + } + + for _, move := range fromTo { + // we need to drop the error here, because the migration has to be idempotent + // and state mv will fail if the state is already migrated + _ = tfClient.StateMv(ctx, move.from, move.to) + } + return nil + }, + }, + } +} diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 80f7fd02d..029bdecb1 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -153,6 +153,17 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, file file.Handler return fmt.Errorf("checking workspace: %w", err) } + // TODO(AB#3248): Remove this migration after we can assume that all existing clusters have been migrated. + var awsZone string + if conf.GetProvider() == cloudprovider.AWS { + awsZone = conf.Provider.AWS.Zone + } + manualMigrations := terraformMigrationAWSNodeGroups(conf.GetProvider(), awsZone) + for _, migration := range manualMigrations { + u.log.Debugf("Adding manual Terraform migration: %s", migration.DisplayName) + u.upgrader.AddManualStateMigration(migration) + } + vars, err := parseTerraformUpgradeVars(cmd, conf, fetcher) if err != nil { return fmt.Errorf("parsing upgrade variables: %w", err) @@ -437,6 +448,7 @@ type cloudUpgrader interface { ApplyTerraformMigrations(ctx context.Context, fileHandler file.Handler, opts upgrade.TerraformUpgradeOptions) error CheckTerraformMigrations(fileHandler file.Handler) error CleanUpTerraformMigrations(fileHandler file.Handler) error + AddManualStateMigration(migration terraform.StateMigration) } func toPtr[T any](v T) *T { diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index 372d85740..5cb30f1d3 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -15,6 +15,7 @@ import ( "github.com/edgelesssys/constellation/v2/cli/internal/clusterid" "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/attestation/variant" "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" @@ -196,6 +197,12 @@ func (u stubUpgrader) ApplyTerraformMigrations(context.Context, file.Handler, up return u.applyTerraformErr } +// AddManualStateMigration is not used in this test. +// TODO(AB#3248): remove this method together with the definition in the interfaces. +func (u stubUpgrader) AddManualStateMigration(_ terraform.StateMigration) { + panic("unused") +} + type stubImageFetcher struct { fetchReferenceErr error } diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index c1f78545c..bb35db504 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -204,6 +204,17 @@ func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fileHandler file.Hand u.log.Debugf("Planning Terraform migrations") + // TODO(AB#3248): Remove this migration after we can assume that all existing clusters have been migrated. + var awsZone string + if csp == cloudprovider.AWS { + awsZone = conf.Provider.AWS.Zone + } + manualMigrations := terraformMigrationAWSNodeGroups(csp, awsZone) + for _, migration := range manualMigrations { + u.log.Debugf("Adding manual Terraform migration: %s", migration.DisplayName) + u.checker.AddManualStateMigration(migration) + } + if err := u.checker.CheckTerraformMigrations(fileHandler); err != nil { return fmt.Errorf("checking workspace: %w", err) } @@ -729,6 +740,7 @@ type upgradeChecker interface { PlanTerraformMigrations(ctx context.Context, opts upgrade.TerraformUpgradeOptions) (bool, error) CheckTerraformMigrations(fileHandler file.Handler) error CleanUpTerraformMigrations(fileHandler file.Handler) error + AddManualStateMigration(migration terraform.StateMigration) } type versionListFetcher interface { diff --git a/cli/internal/cmd/upgradecheck_test.go b/cli/internal/cmd/upgradecheck_test.go index f5507b097..54e8bfb68 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/terraform" "github.com/edgelesssys/constellation/v2/cli/internal/upgrade" "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" "github.com/edgelesssys/constellation/v2/internal/attestation/measurements" @@ -384,6 +385,12 @@ func (u stubUpgradeChecker) CleanUpTerraformMigrations(file.Handler) error { return u.err } +// AddManualStateMigration is not used in this test. +// TODO(AB#3248): remove this method together with the definition in the interfaces. +func (u stubUpgradeChecker) AddManualStateMigration(_ terraform.StateMigration) { + panic("unused") +} + func TestNewCLIVersions(t *testing.T) { someErr := errors.New("some error") minorList := func() versionsapi.List { diff --git a/cli/internal/kubernetes/upgrade.go b/cli/internal/kubernetes/upgrade.go index 10cb8fe8d..e893a9fbc 100644 --- a/cli/internal/kubernetes/upgrade.go +++ b/cli/internal/kubernetes/upgrade.go @@ -94,9 +94,11 @@ type Upgrader struct { helmClient helmInterface imageFetcher imageFetcher outWriter io.Writer - tfUpgrader *upgrade.TerraformUpgrader - log debugLog - upgradeID string + // TODO(AB#3248): Remove this tfClient after we can assume that all existing clusters have been migrated. + tfClient *terraform.Client + tfUpgrader *upgrade.TerraformUpgrader + log debugLog + upgradeID string } // NewUpgrader returns a new Upgrader. @@ -144,6 +146,7 @@ func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog, upgrade if err != nil { return nil, fmt.Errorf("setting up terraform client: %w", err) } + u.tfClient = tfClient tfUpgrader, err := upgrade.NewTerraformUpgrader(tfClient, outWriter) if err != nil { @@ -154,6 +157,12 @@ func NewUpgrader(ctx context.Context, outWriter io.Writer, log debugLog, upgrade return u, nil } +// AddManualStateMigration adds a manual state migration to the Terraform client. +// TODO(AB#3248): Remove this method after we can assume that all existing clusters have been migrated. +func (u *Upgrader) AddManualStateMigration(migration terraform.StateMigration) { + u.tfClient.WithManualStateMigration(migration) +} + // 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 { diff --git a/cli/internal/terraform/terraform.go b/cli/internal/terraform/terraform.go index f5f413bf8..0058512e3 100644 --- a/cli/internal/terraform/terraform.go +++ b/cli/internal/terraform/terraform.go @@ -47,9 +47,10 @@ var ErrTerraformWorkspaceExistsWithDifferentVariables = errors.New("creating clu type Client struct { tf tfInterface - file file.Handler - workingDir string - remove func() + manualStateMigrations []StateMigration + file file.Handler + workingDir string + remove func() } // New sets up a new Client for Terraform. @@ -71,6 +72,12 @@ func New(ctx context.Context, workingDir string) (*Client, error) { }, nil } +// WithManualStateMigration adds a manual state migration to the Client. +func (c *Client) WithManualStateMigration(migration StateMigration) *Client { + c.manualStateMigrations = append(c.manualStateMigrations, migration) + return c +} + // Show reads the default state path and outputs the state. func (c *Client) Show(ctx context.Context) (*tfjson.State, error) { return c.tf.Show(ctx) @@ -105,6 +112,10 @@ func (c *Client) CreateCluster(ctx context.Context, logLevel LogLevel) (CreateOu return CreateOutput{}, fmt.Errorf("terraform init: %w", err) } + if err := c.applyManualStateMigrations(ctx); err != nil { + return CreateOutput{}, fmt.Errorf("apply manual state migrations: %w", err) + } + if err := c.tf.Apply(ctx); err != nil { return CreateOutput{}, fmt.Errorf("terraform apply: %w", err) } @@ -296,6 +307,10 @@ func (c *Client) Plan(ctx context.Context, logLevel LogLevel, planFile string) ( return false, fmt.Errorf("terraform init: %w", err) } + if err := c.applyManualStateMigrations(ctx); err != nil { + return false, fmt.Errorf("apply manual state migrations: %w", err) + } + opts := []tfexec.PlanOption{ tfexec.Out(planFile), } @@ -372,6 +387,23 @@ func GetExecutable(ctx context.Context, workingDir string) (terraform *tfexec.Te return tf, func() { _ = inst.Remove(context.Background()) }, err } +// applyManualStateMigrations applies manual state migrations that are not handled by Terraform due to missing features. +// Each migration is expected to be idempotent. +// This is a temporary solution until we can remove the need for manual state migrations. +func (c *Client) applyManualStateMigrations(ctx context.Context) error { + for _, migration := range c.manualStateMigrations { + if err := migration.Hook(ctx, c.tf); err != nil { + return fmt.Errorf("apply manual state migration %s: %w", migration.DisplayName, err) + } + } + return nil + // expects to be run on initialized workspace + // and only works for AWS + // if migration fails, we expect to either be on a different CSP or that the migration has already been applied + // and we can continue + // c.tf.StateMv(ctx, "module.control_plane.aws_iam_role.this", "module.control_plane.aws_iam_role.control_plane") +} + // writeVars tries to write the Terraform variables file or, if it exists, checks if it is the same as we are expecting. func (c *Client) writeVars(vars Variables) error { if vars == nil { @@ -408,6 +440,13 @@ func (c *Client) setLogLevel(logLevel LogLevel) error { return nil } +// StateMigration is a manual state migration that is not handled by Terraform due to missing features. +// TODO(AB#3248): Remove this after we can assume that all existing clusters have been migrated. +type StateMigration struct { + DisplayName string + Hook func(ctx context.Context, tfClient TFMigrator) error +} + type tfInterface interface { Apply(context.Context, ...tfexec.ApplyOption) error Destroy(context.Context, ...tfexec.DestroyOption) error @@ -417,4 +456,11 @@ type tfInterface interface { ShowPlanFileRaw(ctx context.Context, planPath string, opts ...tfexec.ShowOption) (string, error) SetLog(level string) error SetLogPath(path string) error + TFMigrator +} + +// TFMigrator is an interface for manual terraform state migrations (terraform state mv). +// TODO(AB#3248): Remove this after we can assume that all existing clusters have been migrated. +type TFMigrator interface { + StateMv(ctx context.Context, src, dst string, opts ...tfexec.StateMvCmdOption) error } diff --git a/cli/internal/terraform/terraform_test.go b/cli/internal/terraform/terraform_test.go index f72090706..5acbfb1db 100644 --- a/cli/internal/terraform/terraform_test.go +++ b/cli/internal/terraform/terraform_test.go @@ -1074,6 +1074,7 @@ type stubTerraform struct { setLogPathErr error planJSONErr error showPlanFileErr error + stateMvErr error showState *tfjson.State } @@ -1108,3 +1109,7 @@ func (s *stubTerraform) SetLog(_ string) error { func (s *stubTerraform) SetLogPath(_ string) error { return s.setLogPathErr } + +func (s *stubTerraform) StateMv(_ context.Context, _, _ string, _ ...tfexec.StateMvCmdOption) error { + return s.stateMvErr +}