cli: clean up terraform package (#2256)

* Clean up Terraform pkg

* Add note to Terraform migration functions expecting to be run on initialized workspace

---------

Signed-off-by: Daniel Weiße <dw@edgeless.systems>

cli/internal/cloudcmd/clients.go

@@ -32,13 +32,13 @@ type tfCommonClient interface {
 
 type tfResourceClient interface {
 	tfCommonClient
-	CreateCluster(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.ApplyOutput, error)
+	ApplyCluster(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.ApplyOutput, error)
 	ShowCluster(ctx context.Context, provider cloudprovider.Provider) (terraform.ApplyOutput, error)
 }
 
 type tfIAMClient interface {
 	tfCommonClient
-	ApplyIAMConfig(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.IAMOutput, error)
+	ApplyIAM(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.IAMOutput, error)
 	ShowIAM(ctx context.Context, provider cloudprovider.Provider) (terraform.IAMOutput, error)
 }
 

cli/internal/cloudcmd/clients_test.go

@@ -44,7 +44,7 @@ type stubTerraformClient struct {
 	showErr                error
 }
 
-func (c *stubTerraformClient) CreateCluster(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.ApplyOutput, error) {
+func (c *stubTerraformClient) ApplyCluster(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.ApplyOutput, error) {
 	return terraform.ApplyOutput{
 		IP:     c.ip,
 		Secret: c.initSecret,
@@ -55,7 +55,7 @@ func (c *stubTerraformClient) CreateCluster(_ context.Context, _ cloudprovider.P
 	}, c.createClusterErr
 }
 
-func (c *stubTerraformClient) ApplyIAMConfig(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.IAMOutput, error) {
+func (c *stubTerraformClient) ApplyIAM(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.IAMOutput, error) {
 	return c.iamOutput, c.iamOutputErr
 }
 

cli/internal/cloudcmd/create.go

@@ -229,7 +229,7 @@ func runTerraformCreate(ctx context.Context, cl tfResourceClient, provider cloud
 	}
 
 	defer rollbackOnError(outWriter, &retErr, &rollbackerTerraform{client: cl}, loglevel)
-	tfOutput, err := cl.CreateCluster(ctx, provider, loglevel)
+	tfOutput, err := cl.ApplyCluster(ctx, provider, loglevel)
 	if err != nil {
 		return terraform.ApplyOutput{}, err
 	}
@@ -302,7 +302,7 @@ func (c *Creator) createQEMU(ctx context.Context, cl tfResourceClient, lv libvir
 	// Allow rollback of QEMU Terraform workspace from this point on
 	qemuRollbacker.createdWorkspace = true
 
-	tfOutput, err = cl.CreateCluster(ctx, opts.Provider, opts.TFLogLevel)
+	tfOutput, err = cl.ApplyCluster(ctx, opts.Provider, opts.TFLogLevel)
 	if err != nil {
 		return terraform.ApplyOutput{}, fmt.Errorf("create cluster: %w", err)
 	}

cli/internal/cloudcmd/iam.go

@@ -148,7 +148,7 @@ func (c *IAMCreator) createGCP(ctx context.Context, cl tfIAMClient, opts *IAMCon
 		return IAMOutput{}, err
 	}
 
-	iamOutput, err := cl.ApplyIAMConfig(ctx, cloudprovider.GCP, opts.TFLogLevel)
+	iamOutput, err := cl.ApplyIAM(ctx, cloudprovider.GCP, opts.TFLogLevel)
 	if err != nil {
 		return IAMOutput{}, err
 	}
@@ -175,7 +175,7 @@ func (c *IAMCreator) createAzure(ctx context.Context, cl tfIAMClient, opts *IAMC
 		return IAMOutput{}, err
 	}
 
-	iamOutput, err := cl.ApplyIAMConfig(ctx, cloudprovider.Azure, opts.TFLogLevel)
+	iamOutput, err := cl.ApplyIAM(ctx, cloudprovider.Azure, opts.TFLogLevel)
 	if err != nil {
 		return IAMOutput{}, err
 	}
@@ -203,7 +203,7 @@ func (c *IAMCreator) createAWS(ctx context.Context, cl tfIAMClient, opts *IAMCon
 		return IAMOutput{}, err
 	}
 
-	iamOutput, err := cl.ApplyIAMConfig(ctx, cloudprovider.AWS, opts.TFLogLevel)
+	iamOutput, err := cl.ApplyIAM(ctx, cloudprovider.AWS, opts.TFLogLevel)
 	if err != nil {
 		return IAMOutput{}, err
 	}

cli/internal/terraform/terraform.go

@@ -47,6 +47,18 @@ const (
 	terraformUpgradePlanFile = "plan.zip"
 )
 
+// PrepareIAMUpgradeWorkspace prepares a Terraform workspace for a Constellation IAM upgrade.
+func PrepareIAMUpgradeWorkspace(file file.Handler, path, oldWorkingDir, newWorkingDir, backupDir string) error {
+	if err := prepareUpgradeWorkspace(path, file, oldWorkingDir, newWorkingDir, backupDir); err != nil {
+		return fmt.Errorf("prepare upgrade workspace: %w", err)
+	}
+	// copy the vars file from the old working dir to the new working dir
+	if err := file.CopyFile(filepath.Join(oldWorkingDir, terraformVarsFile), filepath.Join(newWorkingDir, terraformVarsFile)); err != nil {
+		return fmt.Errorf("copying vars file: %w", err)
+	}
+	return nil
+}
+
 // ErrTerraformWorkspaceExistsWithDifferentVariables is returned when existing Terraform files differ from the version the CLI wants to extract.
 var ErrTerraformWorkspaceExistsWithDifferentVariables = errors.New("creating cluster: a Terraform workspace already exists with different variables")
 
@@ -66,7 +78,7 @@ func New(ctx context.Context, workingDir string) (*Client, error) {
 	if err := file.MkdirAll(workingDir); err != nil {
 		return nil, err
 	}
-	tf, remove, err := GetExecutable(ctx, workingDir)
+	tf, remove, err := getExecutable(ctx, workingDir)
 	if err != nil {
 		return nil, err
 	}
@@ -345,104 +357,17 @@ func (c *Client) PrepareUpgradeWorkspace(path, oldWorkingDir, newWorkingDir, bac
 	return c.writeVars(vars)
 }
 
-// PrepareIAMUpgradeWorkspace prepares a Terraform workspace for a Constellation IAM upgrade.
+// ApplyCluster applies the Terraform configuration of the workspace to create or upgrade a Constellation cluster.
-func PrepareIAMUpgradeWorkspace(file file.Handler, path, oldWorkingDir, newWorkingDir, backupDir string) error {
+func (c *Client) ApplyCluster(ctx context.Context, provider cloudprovider.Provider, logLevel LogLevel) (ApplyOutput, error) {
-	if err := prepareUpgradeWorkspace(path, file, oldWorkingDir, newWorkingDir, backupDir); err != nil {
+	if err := c.apply(ctx, logLevel); err != nil {
-		return fmt.Errorf("prepare upgrade workspace: %w", err)
+		return ApplyOutput{}, err
 	}
-	// copy the vars file from the old working dir to the new working dir
-	if err := file.CopyFile(filepath.Join(oldWorkingDir, terraformVarsFile), filepath.Join(newWorkingDir, terraformVarsFile)); err != nil {
-		return fmt.Errorf("copying vars file: %w", err)
-	}
-	return nil
-}
-
-// CreateCluster creates a Constellation cluster using Terraform.
-func (c *Client) CreateCluster(ctx context.Context, provider cloudprovider.Provider, logLevel LogLevel) (ApplyOutput, error) {
-	if err := c.setLogLevel(logLevel); err != nil {
-		return ApplyOutput{}, fmt.Errorf("set terraform log level %s: %w", logLevel.String(), err)
-	}
-
-	if err := c.tf.Init(ctx); err != nil {
-		return ApplyOutput{}, fmt.Errorf("terraform init: %w", err)
-	}
-
-	if err := c.applyManualStateMigrations(ctx); err != nil {
-		return ApplyOutput{}, fmt.Errorf("apply manual state migrations: %w", err)
-	}
-
-	if err := c.tf.Apply(ctx); err != nil {
-		return ApplyOutput{}, fmt.Errorf("terraform apply: %w", err)
-	}
-
 	return c.ShowCluster(ctx, provider)
 }
 
-// ApplyOutput contains the Terraform output values of a cluster creation
+// ApplyIAM applies the Terraform configuration of the workspace to create or upgrade an IAM configuration.
-// or apply operation.
+func (c *Client) ApplyIAM(ctx context.Context, provider cloudprovider.Provider, logLevel LogLevel) (IAMOutput, error) {
-type ApplyOutput struct {
+	if err := c.apply(ctx, logLevel); err != nil {
-	IP                string
-	APIServerCertSANs []string
-	Secret            string
-	UID               string
-	GCP               *GCPApplyOutput
-	Azure             *AzureApplyOutput
-}
-
-// AzureApplyOutput contains the Terraform output values of a terraform apply operation on Microsoft Azure.
-type AzureApplyOutput struct {
-	ResourceGroup            string
-	SubscriptionID           string
-	NetworkSecurityGroupName string
-	LoadBalancerName         string
-	UserAssignedIdentity     string
-	// AttestationURL is the URL of the attestation provider.
-	AttestationURL string
-}
-
-// GCPApplyOutput contains the Terraform output values of a terraform apply operation on GCP.
-type GCPApplyOutput struct {
-	ProjectID  string
-	IPCidrNode string
-	IPCidrPod  string
-}
-
-// IAMOutput contains the output information of the Terraform IAM operations.
-type IAMOutput struct {
-	GCP   GCPIAMOutput
-	Azure AzureIAMOutput
-	AWS   AWSIAMOutput
-}
-
-// GCPIAMOutput contains the output information of the Terraform IAM operation on GCP.
-type GCPIAMOutput struct {
-	SaKey string
-}
-
-// AzureIAMOutput contains the output information of the Terraform IAM operation on Microsoft Azure.
-type AzureIAMOutput struct {
-	SubscriptionID string
-	TenantID       string
-	UAMIID         string
-}
-
-// AWSIAMOutput contains the output information of the Terraform IAM operation on GCP.
-type AWSIAMOutput struct {
-	ControlPlaneInstanceProfile string
-	WorkerNodeInstanceProfile   string
-}
-
-// ApplyIAMConfig creates an IAM configuration using Terraform.
-func (c *Client) ApplyIAMConfig(ctx context.Context, provider cloudprovider.Provider, logLevel LogLevel) (IAMOutput, error) {
-	if err := c.setLogLevel(logLevel); err != nil {
-		return IAMOutput{}, fmt.Errorf("set terraform log level %s: %w", logLevel.String(), err)
-	}
-
-	if err := c.tf.Init(ctx); err != nil {
-		return IAMOutput{}, err
-	}
-
-	if err := c.tf.Apply(ctx); err != nil {
 		return IAMOutput{}, err
 	}
 	return c.ShowIAM(ctx, provider)
@@ -512,45 +437,28 @@ func (c *Client) CleanUpWorkspace() error {
 	return cleanUpWorkspace(c.file, c.workingDir)
 }
 
-// GetExecutable returns a Terraform executable either from the local filesystem,
+func (c *Client) apply(ctx context.Context, logLevel LogLevel) error {
-// or downloads the latest version fulfilling the version constraint.
+	if err := c.setLogLevel(logLevel); err != nil {
-func GetExecutable(ctx context.Context, workingDir string) (terraform *tfexec.Terraform, remove func(), err error) {
+		return fmt.Errorf("set terraform log level %s: %w", logLevel.String(), err)
-	inst := install.NewInstaller()
-
-	version, err := version.NewConstraint(tfVersion)
-	if err != nil {
-		return nil, nil, err
 	}
 
-	constrainedVersions := &releases.Versions{
+	if err := c.tf.Init(ctx); err != nil {
-		Product:     product.Terraform,
+		return fmt.Errorf("terraform init: %w", err)
-		Constraints: version,
-	}
-	installCandidates, err := constrainedVersions.List(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-	if len(installCandidates) == 0 {
-		return nil, nil, fmt.Errorf("no Terraform version found for constraint %s", version)
-	}
-	downloadVersion := installCandidates[len(installCandidates)-1]
-
-	localVersion := &fs.Version{
-		Product:     product.Terraform,
-		Constraints: version,
 	}
 
-	execPath, err := inst.Ensure(ctx, []src.Source{localVersion, downloadVersion})
+	if err := c.applyManualStateMigrations(ctx); err != nil {
-	if err != nil {
+		return fmt.Errorf("apply manual state migrations: %w", err)
-		return nil, nil, err
 	}
 
-	tf, err := tfexec.NewTerraform(workingDir, execPath)
+	if err := c.tf.Apply(ctx); err != nil {
+		return fmt.Errorf("terraform apply: %w", err)
+	}
 
-	return tf, func() { _ = inst.Remove(context.Background()) }, err
+	return nil
 }
 
 // applyManualStateMigrations applies manual state migrations that are not handled by Terraform due to missing features.
+// This functions expects to be run on an initialized Terraform workspace.
 // 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 {
@@ -560,11 +468,6 @@ func (c *Client) applyManualStateMigrations(ctx context.Context) error {
 		}
 	}
 	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.
@@ -612,6 +515,98 @@ type StateMigration struct {
 	Hook        func(ctx context.Context, tfClient TFMigrator) error
 }
 
+// ApplyOutput contains the Terraform output values of a cluster creation
+// or apply operation.
+type ApplyOutput struct {
+	IP                string
+	APIServerCertSANs []string
+	Secret            string
+	UID               string
+	GCP               *GCPApplyOutput
+	Azure             *AzureApplyOutput
+}
+
+// AzureApplyOutput contains the Terraform output values of a terraform apply operation on Microsoft Azure.
+type AzureApplyOutput struct {
+	ResourceGroup            string
+	SubscriptionID           string
+	NetworkSecurityGroupName string
+	LoadBalancerName         string
+	UserAssignedIdentity     string
+	// AttestationURL is the URL of the attestation provider.
+	AttestationURL string
+}
+
+// GCPApplyOutput contains the Terraform output values of a terraform apply operation on GCP.
+type GCPApplyOutput struct {
+	ProjectID  string
+	IPCidrNode string
+	IPCidrPod  string
+}
+
+// IAMOutput contains the output information of the Terraform IAM operations.
+type IAMOutput struct {
+	GCP   GCPIAMOutput
+	Azure AzureIAMOutput
+	AWS   AWSIAMOutput
+}
+
+// GCPIAMOutput contains the output information of the Terraform IAM operation on GCP.
+type GCPIAMOutput struct {
+	SaKey string
+}
+
+// AzureIAMOutput contains the output information of the Terraform IAM operation on Microsoft Azure.
+type AzureIAMOutput struct {
+	SubscriptionID string
+	TenantID       string
+	UAMIID         string
+}
+
+// AWSIAMOutput contains the output information of the Terraform IAM operation on GCP.
+type AWSIAMOutput struct {
+	ControlPlaneInstanceProfile string
+	WorkerNodeInstanceProfile   string
+}
+
+// getExecutable returns a Terraform executable either from the local filesystem,
+// or downloads the latest version fulfilling the version constraint.
+func getExecutable(ctx context.Context, workingDir string) (terraform *tfexec.Terraform, remove func(), err error) {
+	inst := install.NewInstaller()
+
+	version, err := version.NewConstraint(tfVersion)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	constrainedVersions := &releases.Versions{
+		Product:     product.Terraform,
+		Constraints: version,
+	}
+	installCandidates, err := constrainedVersions.List(ctx)
+	if err != nil {
+		return nil, nil, err
+	}
+	if len(installCandidates) == 0 {
+		return nil, nil, fmt.Errorf("no Terraform version found for constraint %s", version)
+	}
+	downloadVersion := installCandidates[len(installCandidates)-1]
+
+	localVersion := &fs.Version{
+		Product:     product.Terraform,
+		Constraints: version,
+	}
+
+	execPath, err := inst.Ensure(ctx, []src.Source{localVersion, downloadVersion})
+	if err != nil {
+		return nil, nil, err
+	}
+
+	tf, err := tfexec.NewTerraform(workingDir, execPath)
+
+	return tf, func() { _ = inst.Remove(context.Background()) }, err
+}
+
 func toStringSlice(in []any) ([]string, error) {
 	out := make([]string, len(in))
 	for i, v := range in {

cli/internal/terraform/terraform_test.go

@@ -449,7 +449,7 @@ func TestCreateCluster(t *testing.T) {
 
 			path := path.Join(tc.pathBase, strings.ToLower(tc.provider.String()))
 			require.NoError(c.PrepareWorkspace(path, tc.vars))
-			tfOutput, err := c.CreateCluster(context.Background(), tc.provider, LogLevelDebug)
+			tfOutput, err := c.ApplyCluster(context.Background(), tc.provider, LogLevelDebug)
 
 			if tc.wantErr {
 				assert.Error(err)
@@ -742,7 +742,7 @@ func TestCreateIAM(t *testing.T) {
 
 			path := path.Join(tc.pathBase, strings.ToLower(tc.provider.String()))
 			require.NoError(c.PrepareWorkspace(path, tc.vars))
-			IAMoutput, err := c.ApplyIAMConfig(context.Background(), tc.provider, LogLevelDebug)
+			IAMoutput, err := c.ApplyIAM(context.Background(), tc.provider, LogLevelDebug)
 
 			if tc.wantErr {
 				assert.Error(err)

cli/internal/upgrade/iammigrate.go

@@ -88,7 +88,7 @@ func (c *IAMMigrateCmd) Plan(ctx context.Context, file file.Handler, outWriter i
 
 // Apply applies the Terraform IAM migrations for the Constellation upgrade.
 func (c *IAMMigrateCmd) Apply(ctx context.Context, fileHandler file.Handler) error {
-	if _, err := c.tf.ApplyIAMConfig(ctx, c.csp, c.logLevel); err != nil {
+	if _, err := c.tf.ApplyIAM(ctx, c.csp, c.logLevel); err != nil {
 		return fmt.Errorf("terraform apply: %w", err)
 	}
 

cli/internal/upgrade/iammigrate_test.go

@@ -97,7 +97,7 @@ func (t *tfClientStub) ShowPlan(_ context.Context, _ terraform.LogLevel, _ io.Wr
 	return nil
 }
 
-func (t *tfClientStub) ApplyIAMConfig(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.IAMOutput, error) {
+func (t *tfClientStub) ApplyIAM(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.IAMOutput, error) {
 	upgradeDir := filepath.Join(constants.UpgradeDir, t.upgradeID, constants.TerraformIAMUpgradeWorkingDir)
 	err := t.file.Remove(filepath.Join(upgradeDir, "terraform.tfvars"))
 	if err != nil {

cli/internal/upgrade/terraform.go

@@ -101,7 +101,7 @@ func (u *TerraformUpgrader) CleanUpTerraformMigrations(upgradeWorkspace string)
 // 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, opts TerraformUpgradeOptions) (terraform.ApplyOutput, error) {
-	tfOutput, err := u.tf.CreateCluster(ctx, opts.CSP, opts.LogLevel)
+	tfOutput, err := u.tf.ApplyCluster(ctx, opts.CSP, opts.LogLevel)
 	if err != nil {
 		return tfOutput, fmt.Errorf("terraform apply: %w", err)
 	}
@@ -181,13 +181,13 @@ type tfClientCommon interface {
 // tfResourceClient is a Terraform client for managing cluster resources.
 type tfResourceClient interface {
 	PrepareUpgradeWorkspace(embeddedPath, oldWorkingDir, newWorkingDir, backupDir string, vars terraform.Variables) error
-	CreateCluster(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.ApplyOutput, error)
+	ApplyCluster(ctx context.Context, provider cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.ApplyOutput, error)
 	tfClientCommon
 }
 
 // tfIAMClient is a Terraform client for managing IAM resources.
 type tfIAMClient interface {
-	ApplyIAMConfig(ctx context.Context, csp cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.IAMOutput, error)
+	ApplyIAM(ctx context.Context, csp cloudprovider.Provider, logLevel terraform.LogLevel) (terraform.IAMOutput, error)
 	tfClientCommon
 }
 

cli/internal/upgrade/terraform_test.go

@@ -318,7 +318,7 @@ func (u *stubTerraformClient) Plan(_ context.Context, _ terraform.LogLevel) (boo
 	return u.hasDiff, u.planErr
 }
 
-func (u *stubTerraformClient) CreateCluster(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.ApplyOutput, error) {
+func (u *stubTerraformClient) ApplyCluster(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.ApplyOutput, error) {
 	return terraform.ApplyOutput{}, u.CreateClusterErr
 }