use skipvariables flag for config provider cmd

cmd/clusterctl/client/client.go

@@ -28,8 +28,8 @@ type Client interface {
 	// GetProvidersConfig returns the list of providers configured for this instance of clusterctl.
 	GetProvidersConfig() ([]Provider, error)
 
-	// GetProviderComponents returns the provider components for a given provider, targetNamespace, watchingNamespace.
-	GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, targetNameSpace, watchingNamespace string) (Components, error)
+	// GetProviderComponents returns the provider components for a given provider with options including targetNamespace, watchingNamespace.
+	GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, options repository.ComponentsOptions) (Components, error)
 
 	// Init initializes a management cluster by adding the requested list of providers.
 	Init(options InitOptions) ([]Components, error)

cmd/clusterctl/client/client_test.go

@@ -72,8 +72,8 @@ func (f fakeClient) GetProvidersConfig() ([]Provider, error) {
 	return f.internalClient.GetProvidersConfig()
 }
 
-func (f fakeClient) GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, targetNameSpace, watchingNamespace string) (Components, error) {
-	return f.internalClient.GetProviderComponents(provider, providerType, targetNameSpace, watchingNamespace)
+func (f fakeClient) GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, options repository.ComponentsOptions) (Components, error) {
+	return f.internalClient.GetProviderComponents(provider, providerType, options)
 }
 
 func (f fakeClient) GetClusterTemplate(options GetClusterTemplateOptions) (Template, error) {
@@ -443,30 +443,16 @@ type fakeComponentClient struct {
 	configClient   config.Client
 }
 
-func (f *fakeComponentClient) Get(version, targetNamespace, watchingNamespace string) (repository.Components, error) {
-	if version == "" {
-		version = f.fakeRepository.DefaultVersion()
+func (f *fakeComponentClient) Get(options repository.ComponentsOptions) (repository.Components, error) {
+	if options.Version == "" {
+		options.Version = f.fakeRepository.DefaultVersion()
 	}
 	path := f.fakeRepository.ComponentsPath()
 
-	content, err := f.fakeRepository.GetFile(version, path)
+	content, err := f.fakeRepository.GetFile(options.Version, path)
 	if err != nil {
 		return nil, err
 	}
 
-	return repository.NewComponents(f.provider, version, content, f.configClient, targetNamespace, watchingNamespace)
-}
-
-func (f *fakeComponentClient) GetWithValues(version, targetNamespace, watchingNamespace string) (repository.Components, error) {
-	if version == "" {
-		version = f.fakeRepository.DefaultVersion()
-	}
-	path := f.fakeRepository.ComponentsPath()
-
-	content, err := f.fakeRepository.GetFile(version, path)
-	if err != nil {
-		return nil, err
-	}
-
-	return repository.NewComponentsWithValues(f.provider, version, content, f.configClient, targetNamespace, watchingNamespace)
+	return repository.NewComponents(f.provider, f.configClient, content, options)
 }

cmd/clusterctl/client/cluster/upgrader.go

@@ -325,7 +325,12 @@ func (u *providerUpgrader) getUpgradeComponents(provider UpgradeItem) (repositor
 		return nil, err
 	}
 
-	components, err := providerRepository.Components().Get(provider.NextVersion, provider.Namespace, provider.WatchedNamespace)
+	options := repository.ComponentsOptions{
+		Version:           provider.NextVersion,
+		TargetNamespace:   provider.Namespace,
+		WatchingNamespace: provider.WatchedNamespace,
+	}
+	components, err := providerRepository.Components().Get(options)
 	if err != nil {
 		return nil, err
 	}

cmd/clusterctl/client/common.go

@@ -25,47 +25,16 @@ import (
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
 )
 
-// getComponentsByNameAndValue is a utility method that returns components
-// for a given provider, targetNamespace, and watchingNamespace.
-func (c *clusterctlClient) getComponentsByNameAndValue(provider string, providerType clusterctlv1.ProviderType, targetNamespace string, watchingNamespace string) (repository.Components, error) {
-
-	// Parse the abbreviated syntax for name[:version]
-	name, version, err := parseProviderName(provider)
-	if err != nil {
-		return nil, err
-	}
-
-	// Gets the provider configuration (that includes the location of the provider repository)
-	providerConfig, err := c.configClient.Providers().Get(name, providerType)
-	if err != nil {
-		return nil, err
-	}
-
-	// Get a client for the provider repository and read the provider components;
-	// during the process, provider components will be processed performing variable substitution, customization of target
-	// and watching namespace etc.
-
-	repository, err := c.repositoryClientFactory(providerConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	components, err := repository.Components().GetWithValues(version, targetNamespace, watchingNamespace)
-	if err != nil {
-		return nil, err
-	}
-	return components, nil
-}
-
 // getComponentsByName is a utility method that returns components
-// for a given provider, targetNamespace, and watchingNamespace.
-func (c *clusterctlClient) getComponentsByName(provider string, providerType clusterctlv1.ProviderType, targetNamespace string, watchingNamespace string) (repository.Components, error) {
+// for a given provider with options including targetNamespace, and watchingNamespace.
+func (c *clusterctlClient) getComponentsByName(provider string, providerType clusterctlv1.ProviderType, options repository.ComponentsOptions) (repository.Components, error) {
 
 	// Parse the abbreviated syntax for name[:version]
 	name, version, err := parseProviderName(provider)
 	if err != nil {
 		return nil, err
 	}
+	options.Version = version
 
 	// Gets the provider configuration (that includes the location of the provider repository)
 	providerConfig, err := c.configClient.Providers().Get(name, providerType)
@@ -77,12 +46,12 @@ func (c *clusterctlClient) getComponentsByName(provider string, providerType clu
 	// during the process, provider components will be processed performing variable substitution, customization of target
 	// and watching namespace etc.
 
-	repository, err := c.repositoryClientFactory(providerConfig)
+	repositoryClientFactory, err := c.repositoryClientFactory(providerConfig)
 	if err != nil {
 		return nil, err
 	}
 
-	components, err := repository.Components().Get(version, targetNamespace, watchingNamespace)
+	components, err := repositoryClientFactory.Components().Get(options)
 	if err != nil {
 		return nil, err
 	}

cmd/clusterctl/client/config.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/version"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
 )
 
 func (c *clusterctlClient) GetProvidersConfig() ([]Provider, error) {
@@ -42,8 +43,9 @@ func (c *clusterctlClient) GetProvidersConfig() ([]Provider, error) {
 	return rr, nil
 }
 
-func (c *clusterctlClient) GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, targetNameSpace, watchingNamespace string) (Components, error) {
-	components, err := c.getComponentsByName(provider, providerType, targetNameSpace, watchingNamespace)
+func (c *clusterctlClient) GetProviderComponents(provider string, providerType clusterctlv1.ProviderType, options repository.ComponentsOptions) (Components, error) {
+	options.SkipVariables = true
+	components, err := c.getComponentsByName(provider, providerType, options)
 	if err != nil {
 		return nil, err
 	}

cmd/clusterctl/client/config_test.go

@@ -23,8 +23,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"k8s.io/utils/pointer"
-
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
@@ -33,6 +31,7 @@ import (
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
 )
 
 func Test_clusterctlClient_GetProvidersConfig(t *testing.T) {
@@ -158,7 +157,11 @@ func Test_clusterctlClient_GetProviderComponents(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			got, err := client.GetProviderComponents(tt.args.provider, capiProviderConfig.Type(), tt.args.targetNameSpace, tt.args.watchingNamespace)
+			options := repository.ComponentsOptions{
+				TargetNamespace:   tt.args.targetNameSpace,
+				WatchingNamespace: tt.args.watchingNamespace,
+			}
+			got, err := client.GetProviderComponents(tt.args.provider, capiProviderConfig.Type(), options)
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 				return
@@ -191,15 +194,19 @@ func Test_getComponentsByName_withEmptyVariables(t *testing.T) {
 		})
 
 	// Create a fake cluster, eventually adding some existing runtime objects to it.
-	cluster1 := newFakeCluster("cluster1", config1).WithObjs()
+	cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1).WithObjs()
 
 	// Create a new fakeClient that allows to execute tests on the fake config,
 	// the fake repositories and the fake cluster.
 	client := newFakeClient(config1).
 		WithRepository(repository1).
 		WithCluster(cluster1)
 
-	components, err := client.GetProviderComponents(repository1Config.Name(), repository1Config.Type(), "ns1", "")
+	options := repository.ComponentsOptions{
+		TargetNamespace:   "ns1",
+		WatchingNamespace: "",
+	}
+	components, err := client.GetProviderComponents(repository1Config.Name(), repository1Config.Type(), options)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(len(components.Variables())).To(Equal(1))
 	g.Expect(components.Name()).To(Equal("p1"))

cmd/clusterctl/client/init.go

@@ -23,6 +23,7 @@ import (
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
 )
 
@@ -231,8 +232,11 @@ func (c *clusterctlClient) addToInstaller(options addToInstallerOptions, provide
 			}
 			continue
 		}
-
-		components, err := c.getComponentsByNameAndValue(provider, providerType, options.targetNamespace, options.watchingNamespace)
+		componentsOptions := repository.ComponentsOptions{
+			TargetNamespace:   options.targetNamespace,
+			WatchingNamespace: options.watchingNamespace,
+		}
+		components, err := c.getComponentsByName(provider, providerType, componentsOptions)
 		if err != nil {
 			return errors.Wrapf(err, "failed to get provider components for the %q provider", provider)
 		}

cmd/clusterctl/client/repository/components.go

@@ -174,44 +174,38 @@ func (c *components) Yaml() ([]byte, error) {
 	return util.FromUnstructured(objs)
 }
 
-// NewComponentsWithValues returns a new objects embedding a component YAML file
-//
-// It is important to notice that clusterctl applies a set of processing steps to the “raw” component YAML read
-// from the provider repositories:
-// 1. Checks for all the variables in the component YAML file and replace with corresponding config values
-// 2. Ensure all the provider components are deployed in the target namespace (apply only to namespaced objects)
-// 3. Ensure all the ClusterRoleBinding which are referencing namespaced objects have the name prefixed with the namespace name
-// 4. Set the watching namespace for the provider controller
-// 5. Adds labels to all the components in order to allow easy identification of the provider objects
-func NewComponentsWithValues(provider config.Provider, version string, rawyaml []byte, configClient config.Client, targetNamespace, watchingNamespace string) (*components, error) {
-	// inspect the yaml read from the repository for variables
-	variables := inspectVariables(rawyaml)
-
-	// Replace variables with corresponding values read from the config
-	yaml, err := replaceVariables(rawyaml, variables, configClient.Variables())
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to perform variable substitution")
-	}
-
-	return processComponentsYaml(provider, version, yaml, variables, configClient, targetNamespace, watchingNamespace)
+// ComponentsOptions is the inputs needed by the NewComponents
+type ComponentsOptions struct {
+	Version           string
+	TargetNamespace   string
+	WatchingNamespace string
+	SkipVariables     bool
 }
 
 // NewComponents returns a new objects embedding a component YAML file
 //
 // It is important to notice that clusterctl applies a set of processing steps to the “raw” component YAML read
 // from the provider repositories:
-// 1. Checks for all the variables in the component YAML file without replacing the values
-// 2. Ensure all the provider components are deployed in the target namespace (apply only to namespaced objects)
-// 3. Ensure all the ClusterRoleBinding which are referencing namespaced objects have the name prefixed with the namespace name
-// 4. Set the watching namespace for the provider controller
-// 5. Adds labels to all the components in order to allow easy identification of the provider objects
-func NewComponents(provider config.Provider, version string, rawyaml []byte, configClient config.Client, targetNamespace, watchingNamespace string) (*components, error) {
+// 1. Checks for all the variables in the component YAML file and replace with corresponding config values
+// 2. The variables replacement can be skipped using the SkipVariables flag in the input options
+// 3. Ensure all the provider components are deployed in the target namespace (apply only to namespaced objects)
+// 4. Ensure all the ClusterRoleBinding which are referencing namespaced objects have the name prefixed with the namespace name
+// 5. Set the watching namespace for the provider controller
+// 6. Adds labels to all the components in order to allow easy identification of the provider objects
+func NewComponents(provider config.Provider, configClient config.Client, rawyaml []byte, options ComponentsOptions) (*components, error) {
 	// Inspect the yaml read from the repository for variables.
 	variables := inspectVariables(rawyaml)
-	return processComponentsYaml(provider, version, rawyaml, variables, configClient, targetNamespace, watchingNamespace)
-}
 
-func processComponentsYaml(provider config.Provider, version string, yaml []byte, variables []string, configClient config.Client, targetNamespace, watchingNamespace string) (*components, error) {
+	yaml := rawyaml
+	var err error
+	if !options.SkipVariables {
+		// Replace variables with corresponding values read from the config
+		yaml, err = replaceVariables(rawyaml, variables, configClient.Variables())
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to perform variable substitution")
+		}
+	}
+
 	// Transform the yaml in a list of objects, so following transformation can work on typed objects (instead of working on a string/slice of bytes)
 	objs, err := util.ToUnstructured(yaml)
 	if err != nil {
@@ -251,24 +245,24 @@ func processComponentsYaml(provider config.Provider, version string, yaml []byte
 	// if targetNamespace is not specified, then defaultTargetNamespace is used. In case both targetNamespace and defaultTargetNamespace
 	// are empty, an error is returned
 
-	if targetNamespace == "" {
-		targetNamespace = defaultTargetNamespace
+	if options.TargetNamespace == "" {
+		options.TargetNamespace = defaultTargetNamespace
 	}
 
-	if targetNamespace == "" {
+	if options.TargetNamespace == "" {
 		return nil, errors.New("target namespace can't be defaulted. Please specify a target namespace")
 	}
 
 	// add a Namespace object if missing (ensure the targetNamespace will be created)
-	instanceObjs = addNamespaceIfMissing(instanceObjs, targetNamespace)
+	instanceObjs = addNamespaceIfMissing(instanceObjs, options.TargetNamespace)
 
 	// fix Namespace name in all the objects
-	instanceObjs = fixTargetNamespace(instanceObjs, targetNamespace)
+	instanceObjs = fixTargetNamespace(instanceObjs, options.TargetNamespace)
 
 	// ensures all the ClusterRole and ClusterRoleBinding have the name prefixed with the namespace name and that
 	// all the clusterRole/clusterRoleBinding namespaced subjects refers to targetNamespace
 	// Nb. Making all the RBAC rules "namespaced" is required for supporting multi-tenancy
-	instanceObjs, err = fixRBAC(instanceObjs, targetNamespace)
+	instanceObjs, err = fixRBAC(instanceObjs, options.TargetNamespace)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to fix ClusterRoleBinding names")
 	}
@@ -281,8 +275,8 @@ func processComponentsYaml(provider config.Provider, version string, yaml []byte
 	}
 
 	// if the requested watchingNamespace is different from the defaultWatchingNamespace, fix it
-	if defaultWatchingNamespace != watchingNamespace {
-		instanceObjs, err = fixWatchNamespace(instanceObjs, watchingNamespace)
+	if defaultWatchingNamespace != options.WatchingNamespace {
+		instanceObjs, err = fixWatchNamespace(instanceObjs, options.WatchingNamespace)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to set watching namespace")
 		}
@@ -299,11 +293,11 @@ func processComponentsYaml(provider config.Provider, version string, yaml []byte
 
 	return &components{
 		Provider:          provider,
-		version:           version,
+		version:           options.Version,
 		variables:         variables,
 		images:            images,
-		targetNamespace:   targetNamespace,
-		watchingNamespace: watchingNamespace,
+		targetNamespace:   options.TargetNamespace,
+		watchingNamespace: options.WatchingNamespace,
 		instanceObjs:      instanceObjs,
 		sharedObjs:        sharedObjs,
 	}, nil