From b040ae65da713141cefd83def37d9425b99204ca Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 27 Jun 2022 11:32:20 +0800 Subject: [PATCH] [Backport release-1.4] Fix: provider can't be added since 1.4 as context abused && Feat: add cache for remote terraform module in vela show (#4263) * Fix: provider can't be added since 1.4 as context abused Signed-off-by: Jianbo Sun (cherry picked from commit b05fb26418bcd5739084246a01c4bb1d8ada980c) * Feat: add cache for remote terraform module in vela show Signed-off-by: Jianbo Sun (cherry picked from commit 4722028530aa136746a28860a61457cd29ee1fc8) * Fix: add message for terraform resource in error state Signed-off-by: Jianbo Sun (cherry picked from commit 438145b12e8dfb40a9c85e559cfcd6c145341dc6) Co-authored-by: Jianbo Sun --- .../v1alpha2/application/apply.go | 2 +- .../v1alpha2/application/generator.go | 2 +- pkg/controller/utils/capability.go | 35 ++++++------ ...inux_test.go => capability_config_test.go} | 53 +++---------------- pkg/utils/config/application.go | 3 +- references/cli/provider.go | 6 ++- references/plugins/reference_test.go | 3 +- 7 files changed, 33 insertions(+), 71 deletions(-) rename pkg/controller/utils/{capability_linux_test.go => capability_config_test.go} (64%) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go index 604f39b64..9e17eb089 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go @@ -305,12 +305,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 } diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go index 53d23ad42..99472c4d5 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go @@ -188,7 +188,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 } diff --git a/pkg/controller/utils/capability.go b/pkg/controller/utils/capability.go index 0ea5fc9ea..a3033274f 100644 --- a/pkg/controller/utils/capability.go +++ b/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 } diff --git a/pkg/controller/utils/capability_linux_test.go b/pkg/controller/utils/capability_config_test.go similarity index 64% rename from pkg/controller/utils/capability_linux_test.go rename to pkg/controller/utils/capability_config_test.go index e269a4f0d..310e75ac7 100644 --- a/pkg/controller/utils/capability_linux_test.go +++ b/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 != "" { diff --git a/pkg/utils/config/application.go b/pkg/utils/config/application.go index 927749ec2..7a8721cb4 100644 --- a/pkg/utils/config/application.go +++ b/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) diff --git a/references/cli/provider.go b/references/cli/provider.go index dfb8962fe..6fddc4df5 100644 --- a/references/cli/provider.go +++ b/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) diff --git a/references/plugins/reference_test.go b/references/plugins/reference_test.go index 443538848..a24da25c7 100644 --- a/references/plugins/reference_test.go +++ b/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", }, },