Fix: provider can't be added since 1.4 as context abused && Feat: add cache for remote terraform module in vela show (#4256)

* Fix: provider can't be added since 1.4 as context abused

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

* Feat: add cache for remote terraform module in vela show

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

* Fix: add message for terraform resource in error state

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

pkg/controller/core.oam.dev/v1alpha2/application/apply.go

@@ -308,12 +308,12 @@ func setStatus(status *common.ApplicationComponentStatus, observedGeneration, ge
 		}
 		return true
 	}
+	status.Message = message
 	if !isLatest() || state != terraformtypes.Available {
 		status.Healthy = false
 		return false
 	}
 	status.Healthy = true
-	status.Message = message
 	return true
 }
 

pkg/controller/core.oam.dev/v1alpha2/application/generator.go

@@ -199,7 +199,7 @@ func convertStepProperties(step *v1beta1.WorkflowStep, app *v1beta1.Application)
 }
 
 func checkDependsOnValidComponent(dependsOnComponentNames, allComponentNames []string) (string, bool) {
-	// does not depends on other components
+	// does not depend on other components
 	if dependsOnComponentNames == nil {
 		return "", true
 	}

pkg/controller/utils/capability.go

@@ -30,7 +30,7 @@ import (
 	"cuelang.org/go/cue/build"
 	"github.com/getkin/kin-openapi/openapi3"
 	"github.com/pkg/errors"
-	git "gopkg.in/src-d/go-git.v4"
+	"gopkg.in/src-d/go-git.v4"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -215,25 +215,26 @@ func GetOpenAPISchemaFromTerraformComponentDefinition(configuration string) ([]b
 
 // GetTerraformConfigurationFromRemote gets Terraform Configuration(HCL)
 func GetTerraformConfigurationFromRemote(name, remoteURL, remotePath string) (string, error) {
-	tmpPath := filepath.Join("./tmp/terraform", name)
-	// Check if the directory exists. If yes, remove it.
-	if _, err := os.Stat(tmpPath); err == nil {
-		err := os.RemoveAll(tmpPath)
-		if err != nil {
-			return "", errors.Wrap(err, "failed to remove the directory")
-		}
-	}
-	_, err := git.PlainClone(tmpPath, false, &git.CloneOptions{
-		URL:      remoteURL,
-		Progress: nil,
-	})
+	userHome, err := os.UserHomeDir()
 	if err != nil {
 		return "", err
 	}
+	cachePath := filepath.Join(userHome, ".vela", "terraform", name)
+	// Check if the directory exists. If yes, remove it.
+	entities, err := os.ReadDir(cachePath)
+	if err != nil || len(entities) == 0 {
+		fmt.Printf("loading terraform module %s into %s from %s\n", name, cachePath, remoteURL)
+		if _, err = git.PlainClone(cachePath, false, &git.CloneOptions{
+			URL:      remoteURL,
+			Progress: os.Stdout,
+		}); err != nil {
+			return "", err
+		}
+	}
 
-	tfPath := filepath.Join(tmpPath, remotePath, "variables.tf")
+	tfPath := filepath.Join(cachePath, remotePath, "variables.tf")
 	if _, err := os.Stat(tfPath); err != nil {
-		tfPath = filepath.Join(tmpPath, remotePath, "main.tf")
+		tfPath = filepath.Join(cachePath, remotePath, "main.tf")
 		if _, err := os.Stat(tfPath); err != nil {
 			return "", errors.Wrap(err, "failed to find main.tf or variables.tf in Terraform configurations of the remote repository")
 		}
@@ -242,10 +243,6 @@ func GetTerraformConfigurationFromRemote(name, remoteURL, remotePath string) (st
 	if err != nil {
 		return "", errors.Wrap(err, "failed to read Terraform configuration")
 	}
-	if err := os.RemoveAll(tmpPath); err != nil {
-		return "", err
-	}
-
 	return string(conf), nil
 }
 

pkg/controller/utils/capability_config_test.go

@@ -17,24 +17,16 @@
 package utils
 
 import (
-	"context"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
-	. "github.com/agiledragon/gomonkey/v2"
-
-	"github.com/pkg/errors"
-	"gopkg.in/src-d/go-git.v4"
 	"gotest.tools/assert"
 )
 
 func TestGetTerraformConfigurationFromRemote(t *testing.T) {
-	// If you hit a panic on macOS as below, please fix it by referencing https://github.com/eisenxp/macos-golink-wrapper.
-	// panic: permission denied [recovered]
-	//    panic: permission denied
 	type want struct {
 		config string
 		errMsg string
@@ -46,8 +38,6 @@ func TestGetTerraformConfigurationFromRemote(t *testing.T) {
 		path         string
 		data         []byte
 		variableFile string
-		// mockWorkingPath will create `/tmp/terraform`
-		mockWorkingPath bool
 	}
 	cases := map[string]struct {
 		args args
@@ -57,7 +47,7 @@ func TestGetTerraformConfigurationFromRemote(t *testing.T) {
 			args: args{
 				name: "valid",
 				url:  "https://github.com/kubevela-contrib/terraform-modules.git",
-				path: "",
+				path: "unittest/",
 				data: []byte(`
 variable "aaa" {
 	type = list(object({
@@ -85,7 +75,7 @@ variable "aaa" {
 			args: args{
 				name: "aws-subnet",
 				url:  "https://github.com/kubevela-contrib/terraform-modules.git",
-				path: "aws/subnet",
+				path: "unittest/aws/subnet",
 				data: []byte(`
 variable "aaa" {
 	type = list(object({
@@ -109,47 +99,20 @@ variable "aaa" {
 }`,
 			},
 		},
-		"working path exists": {
-			args: args{
-				variableFile:    "main.tf",
-				mockWorkingPath: true,
-			},
-			want: want{
-				errMsg: "failed to remove the directory",
-			},
-		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			if tc.args.mockWorkingPath {
-				err := os.MkdirAll("./tmp/terraform", 0755)
-				assert.NilError(t, err)
-				defer os.RemoveAll("./tmp/terraform")
-				patch1 := ApplyFunc(os.Remove, func(_ string) error {
-					return errors.New("failed")
-				})
-				defer patch1.Reset()
-				patch2 := ApplyFunc(os.Open, func(_ string) (*os.File, error) {
-					return nil, errors.New("failed")
-				})
-				defer patch2.Reset()
-			}
-
-			patch := ApplyFunc(git.PlainCloneContext, func(ctx context.Context, path string, isBare bool, o *git.CloneOptions) (*git.Repository, error) {
-				var tmpPath string
-				if tc.args.path != "" {
-					tmpPath = filepath.Join("./tmp/terraform", tc.args.name, tc.args.path)
-				} else {
-					tmpPath = filepath.Join("./tmp/terraform", tc.args.name)
-				}
+			home, _ := os.UserHomeDir()
+			path := filepath.Join(home, ".vela", "terraform")
+			tmpPath := filepath.Join(path, tc.args.name, tc.args.path)
+			if len(tc.args.data) > 0 {
 				err := os.MkdirAll(tmpPath, os.ModePerm)
 				assert.NilError(t, err)
 				err = ioutil.WriteFile(filepath.Clean(filepath.Join(tmpPath, tc.args.variableFile)), tc.args.data, 0644)
 				assert.NilError(t, err)
-				return nil, nil
-			})
-			defer patch.Reset()
+			}
+			defer os.RemoveAll(tmpPath)
 
 			conf, err := GetTerraformConfigurationFromRemote(tc.args.name, tc.args.url, tc.args.path)
 			if tc.want.errMsg != "" {

pkg/utils/config/application.go

@@ -35,7 +35,6 @@ import (
 )
 
 const (
-	errAuthenticateProvider   = "failed to authenticate Terraform cloud provider %s for %s"
 	errProviderExists         = "terraform provider %s for %s already exists"
 	errDeleteProvider         = "failed to delete Terraform Provider %s err: %w"
 	errCouldNotDeleteProvider = "the Terraform Provider %s could not be disabled because it was created by enabling a Terraform provider or was manually created"
@@ -54,7 +53,7 @@ func CreateApplication(ctx context.Context, k8sClient client.Client, name, compo
 	if strings.HasPrefix(componentType, types.TerraformComponentPrefix) {
 		existed, err := IsTerraformProviderExisted(ctx, k8sClient, name)
 		if err != nil {
-			return errors.Wrapf(err, errAuthenticateProvider, name, componentType)
+			return errors.Wrapf(err, errCheckProviderExistence, name)
 		}
 		if existed {
 			return fmt.Errorf(errProviderExists, name, componentType)

references/cli/provider.go

@@ -166,9 +166,12 @@ func prepareProviderAddSubCommand(c common.Args, ioStreams cmdutil.IOStreams) ([
 				return nil, err
 			}
 			for _, p := range parameters {
+				// TODO(wonderflow): make the provider default name to be unique but keep the compatiblility as some Application didn't specify the name,
+				// now it's “default” for every one, the name will conflict if we have more than one cloud provider.
 				cmd.Flags().String(p.Name, fmt.Sprint(p.Default), p.Usage)
 			}
 			cmd.RunE = func(cmd *cobra.Command, args []string) error {
+				newContext := context.Background()
 				name, err := cmd.Flags().GetString(providerNameParam)
 				if err != nil || name == "" {
 					return fmt.Errorf("must specify a name for the Terraform Cloud Provider %s", providerType)
@@ -188,8 +191,7 @@ func prepareProviderAddSubCommand(c common.Args, ioStreams cmdutil.IOStreams) ([
 				if err != nil {
 					return fmt.Errorf(errAuthenticateProvider, providerType, err)
 				}
-
-				if err := config.CreateApplication(ctx, k8sClient, name, providerType, string(data), config.UIParam{}); err != nil {
+				if err := config.CreateApplication(newContext, k8sClient, name, providerType, string(data), config.UIParam{}); err != nil {
 					return fmt.Errorf(errAuthenticateProvider, providerType, err)
 				}
 				ioStreams.Infof("Successfully authenticate provider %s for %s\n", name, providerType)

references/plugins/reference_test.go

@@ -430,7 +430,8 @@ variable "acl" {
 		"configuration is in git remote": {
 			args: args{
 				cap: types.Capability{
-					TerraformConfiguration: "https://github.com/zzxwill/terraform-alibaba-eip.git",
+					Name:                   "ecs",
+					TerraformConfiguration: "https://github.com/wonderflow/terraform-alicloud-ecs-instance.git",
 					ConfigurationType:      "remote",
 				},
 			},