🐛 Init images skip variables

cmd/clusterctl/client/client_test.go

@@ -353,7 +353,7 @@ func (f fakeRepositoryClient) Components() repository.ComponentsClient {
 }
 
 func (f fakeRepositoryClient) Templates(version string) repository.TemplateClient {
-	// use a fakeComponentClient (instead of the internal client used in other fake objects) we can de deterministic on what is returned (e.g. avoid interferences from overrides)
+	// use a fakeTemplateClient (instead of the internal client used in other fake objects) we can de deterministic on what is returned (e.g. avoid interferences from overrides)
 	return &fakeTemplateClient{
 		version:               version,
 		fakeRepository:        f.fakeRepository,
@@ -362,7 +362,7 @@ func (f fakeRepositoryClient) Templates(version string) repository.TemplateClien
 }
 
 func (f fakeRepositoryClient) Metadata(version string) repository.MetadataClient {
-	// use a fakeComponentClient (instead of the internal client used in other fake objects) we can de deterministic on what is returned (e.g. avoid interferences from overrides)
+	// use a fakeMetadataClient (instead of the internal client used in other fake objects) we can de deterministic on what is returned (e.g. avoid interferences from overrides)
 	return &fakeMetadataClient{
 		version:        version,
 		fakeRepository: f.fakeRepository,

cmd/clusterctl/client/config.go

@@ -94,7 +94,7 @@ type GetClusterTemplateOptions struct {
 	// It can be set through the cli flag, WORKER_MACHINE_COUNT environment variable or will default to 0
 	WorkerMachineCount *int64
 
-	// listVariablesOnly sets the GetClusterTemplate method to return the list of variables expected by the template
+	// ListVariablesOnly sets the GetClusterTemplate method to return the list of variables expected by the template
 	// without executing any further processing.
 	ListVariablesOnly bool
 }

cmd/clusterctl/client/init.go

@@ -60,6 +60,10 @@ type InitOptions struct {
 
 	// LogUsageInstructions instructs the init command to print the usage instructions in case of first run.
 	LogUsageInstructions bool
+
+	// skipVariables skips variable parsing in the provider components yaml.
+	// It is set to true for listing images of provider components.
+	skipVariables bool
 }
 
 // Init initializes a management cluster by adding the requested list of providers.
@@ -142,6 +146,9 @@ func (c *clusterctlClient) InitImages(options InitOptions) ([]string, error) {
 	// a bootstrap provider and a control-plane provider (if not already explicitly requested by the user)
 	c.addDefaultProviders(cluster, &options)
 
+	// skip variable parsing when listing images
+	options.skipVariables = true
+
 	// create an installer service, add the requested providers to the install queue and then perform validation
 	// of the target state of the management cluster before starting the installation.
 	installer, err := c.setupInstaller(cluster, options)
@@ -169,6 +176,7 @@ func (c *clusterctlClient) setupInstaller(cluster cluster.Client, options InitOp
 		installer:         installer,
 		targetNamespace:   options.TargetNamespace,
 		watchingNamespace: options.WatchingNamespace,
+		skipVariables:     options.skipVariables,
 	}
 
 	if options.CoreProvider != "" {
@@ -220,6 +228,7 @@ type addToInstallerOptions struct {
 	installer         cluster.ProviderInstaller
 	targetNamespace   string
 	watchingNamespace string
+	skipVariables     bool
 }
 
 // addToInstaller adds the components to the install queue and checks that the actual provider type match the target group
@@ -235,6 +244,7 @@ func (c *clusterctlClient) addToInstaller(options addToInstallerOptions, provide
 		componentsOptions := repository.ComponentsOptions{
 			TargetNamespace:   options.targetNamespace,
 			WatchingNamespace: options.watchingNamespace,
+			SkipVariables:     options.skipVariables,
 		}
 		components, err := c.getComponentsByName(provider, providerType, componentsOptions)
 		if err != nil {

cmd/clusterctl/client/init_test.go

@@ -29,19 +29,29 @@ import (
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/util"
 )
 
-// TODO: Refactor to actually test InitImages
 func Test_clusterctlClient_InitImages(t *testing.T) {
 	type field struct {
 		client *fakeClient
 	}
 
 	type args struct {
+		kubeconfigContext      string
 		coreProvider           string
 		bootstrapProvider      []string
 		controlPlaneProvider   []string
 		infrastructureProvider []string
 	}
 
+	// create a config variables client which does not have the value for
+	// SOME_VARIABLE as expected in the infra components YAML
+	config := newFakeConfig().
+		WithVar("ANOTHER_VARIABLE", "value").
+		WithProvider(capiProviderConfig).
+		WithProvider(infraProviderConfig)
+
+	cluster := fakeCluster(config, fakeRepositories(config))
+	fc := fakeClusterCtlClient(config, fakeRepositories(config), []*fakeClusterClient{cluster})
+
 	tests := []struct {
 		name           string
 		field          field
@@ -54,18 +64,38 @@ func Test_clusterctlClient_InitImages(t *testing.T) {
 			field: field{
 				client: fakeEmptyCluster(),
 			},
-			args:           args{},
+			args: args{
+				kubeconfigContext: "does-not-exist",
+			},
 			expectedImages: []string{},
 			wantErr:        true,
 		},
+		{
+			name: "returns list of images even if component variable values are not found",
+			field: field{
+				client: fc,
+			},
+			args: args{
+				coreProvider:           "",  // with an empty cluster, a core provider should be added automatically
+				bootstrapProvider:      nil, // with an empty cluster, a bootstrap provider should be added automatically
+				controlPlaneProvider:   nil, // with an empty cluster, a control plane provider should be added automatically
+				infrastructureProvider: []string{"infra"},
+				kubeconfigContext:      "mgmt-context",
+			},
+			expectedImages: []string{
+				"gcr.io/kubebuilder/kube-rbac-proxy:v0.4.1",
+				"us.gcr.io/k8s-artifacts-prod/cluster-api-aws/cluster-api-aws-controller:v0.5.3",
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
 			got, err := tt.field.client.InitImages(InitOptions{
-				Kubeconfig:              Kubeconfig{Path: "kubeconfig", Context: "does-not-exist"},
+				Kubeconfig:              Kubeconfig{Path: "kubeconfig", Context: tt.args.kubeconfigContext},
 				CoreProvider:            tt.args.coreProvider,
 				BootstrapProviders:      tt.args.bootstrapProvider,
 				ControlPlaneProviders:   tt.args.controlPlaneProvider,
@@ -77,11 +107,22 @@ func Test_clusterctlClient_InitImages(t *testing.T) {
 			}
 			g.Expect(err).NotTo(HaveOccurred())
 			g.Expect(got).To(HaveLen(len(tt.expectedImages)))
+			g.Expect(got).To(ConsistOf(tt.expectedImages))
 		})
 	}
 }
 
 func Test_clusterctlClient_Init(t *testing.T) {
+	// create a config variables client which does not have the value for
+	// SOME_VARIABLE as expected in the infra components YAML
+	fconfig := newFakeConfig().
+		WithVar("ANOTHER_VARIABLE", "value").
+		WithProvider(capiProviderConfig).
+		WithProvider(infraProviderConfig)
+	frepositories := fakeRepositories(fconfig)
+	fcluster := fakeCluster(fconfig, frepositories)
+	fclient := fakeClusterCtlClient(fconfig, frepositories, []*fakeClusterClient{fcluster})
+
 	type field struct {
 		client *fakeClient
 		hasCRD bool
@@ -109,6 +150,19 @@ func Test_clusterctlClient_Init(t *testing.T) {
 		want    []want
 		wantErr bool
 	}{
+		{
+			name: "returns error if variables are not available",
+			field: field{
+				client: fclient,
+			},
+			args: args{
+				coreProvider:           "",  // with an empty cluster, a core provider should be added automatically
+				bootstrapProvider:      nil, // with an empty cluster, a bootstrap provider should be added automatically
+				controlPlaneProvider:   nil, // with an empty cluster, a control plane provider should be added automatically
+				infrastructureProvider: []string{"infra"},
+			},
+			wantErr: true,
+		},
 		{
 			name: "Init (with an empty cluster) with default provider versions",
 			field: field{
@@ -420,14 +474,46 @@ var (
 
 // clusterctl client for an empty management cluster (with repository setup for capi, bootstrap and infra provider)
 func fakeEmptyCluster() *fakeClient {
-	config1 := newFakeConfig().
-		WithVar("var", "value").
-		WithProvider(capiProviderConfig).
-		WithProvider(bootstrapProviderConfig).
-		WithProvider(controlPlaneProviderConfig).
-		WithProvider(infraProviderConfig)
+	// create a config variables client which contains the value for the
+	// variable required
+	config1 := fakeConfig(
+		[]config.Provider{capiProviderConfig, bootstrapProviderConfig, controlPlaneProviderConfig, infraProviderConfig},
+		map[string]string{"SOME_VARIABLE": "value"},
+	)
+
+	// fake repository for capi, bootstrap and infra provider (matching provider's config)
+	repositories := fakeRepositories(config1)
+	// fake empty cluster from fake repository for capi, bootstrap and infra
+	// provider (matching provider's config)
+	cluster1 := fakeCluster(config1, repositories)
+
+	client := fakeClusterCtlClient(config1, repositories, []*fakeClusterClient{cluster1})
+	return client
+}
 
-	repository1 := newFakeRepository(capiProviderConfig, config1).
+func fakeConfig(providers []config.Provider, variables map[string]string) *fakeConfigClient {
+	config := newFakeConfig()
+	for _, p := range providers {
+		config = config.WithProvider(p)
+	}
+	for k, v := range variables {
+		config = config.WithVar(k, v)
+	}
+	return config
+}
+
+func fakeCluster(config *fakeConfigClient, repos []*fakeRepositoryClient) *fakeClusterClient {
+	cluster := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config)
+	for _, r := range repos {
+		cluster = cluster.WithRepository(r)
+	}
+	return cluster
+}
+
+// fakeRepositories returns a base set of repositories for the different types
+// of providers.
+func fakeRepositories(config *fakeConfigClient) []*fakeRepositoryClient {
+	repository1 := newFakeRepository(capiProviderConfig, config).
 		WithPaths("root", "components.yaml").
 		WithDefaultVersion("v1.0.0").
 		WithFile("v1.0.0", "components.yaml", componentsYAML("ns1")).
@@ -442,7 +528,7 @@ func fakeEmptyCluster() *fakeClient {
 				{Major: 1, Minor: 1, Contract: "v1alpha3"},
 			},
 		})
-	repository2 := newFakeRepository(bootstrapProviderConfig, config1).
+	repository2 := newFakeRepository(bootstrapProviderConfig, config).
 		WithPaths("root", "components.yaml").
 		WithDefaultVersion("v2.0.0").
 		WithFile("v2.0.0", "components.yaml", componentsYAML("ns2")).
@@ -457,7 +543,7 @@ func fakeEmptyCluster() *fakeClient {
 				{Major: 2, Minor: 1, Contract: "v1alpha3"},
 			},
 		})
-	repository3 := newFakeRepository(controlPlaneProviderConfig, config1).
+	repository3 := newFakeRepository(controlPlaneProviderConfig, config).
 		WithPaths("root", "components.yaml").
 		WithDefaultVersion("v2.0.0").
 		WithFile("v2.0.0", "components.yaml", componentsYAML("ns3")).
@@ -472,39 +558,34 @@ func fakeEmptyCluster() *fakeClient {
 				{Major: 2, Minor: 1, Contract: "v1alpha3"},
 			},
 		})
-	repository4 := newFakeRepository(infraProviderConfig, config1).
+	repository4 := newFakeRepository(infraProviderConfig, config).
 		WithPaths("root", "components.yaml").
 		WithDefaultVersion("v3.0.0").
-		WithFile("v3.0.0", "components.yaml", componentsYAML("ns4")).
+		WithFile("v3.0.0", "components.yaml", infraComponentsYAML("ns4")).
 		WithMetadata("v3.0.0", &clusterctlv1.Metadata{
 			ReleaseSeries: []clusterctlv1.ReleaseSeries{
 				{Major: 3, Minor: 0, Contract: "v1alpha3"},
 			},
 		}).
-		WithFile("v3.1.0", "components.yaml", componentsYAML("ns4")).
+		WithFile("v3.1.0", "components.yaml", infraComponentsYAML("ns4")).
 		WithMetadata("v3.1.0", &clusterctlv1.Metadata{
 			ReleaseSeries: []clusterctlv1.ReleaseSeries{
 				{Major: 3, Minor: 1, Contract: "v1alpha3"},
 			},
 		}).
 		WithFile("v3.0.0", "cluster-template.yaml", templateYAML("ns4", "test"))
 
-	cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1).
-		// fake repository for capi, bootstrap and infra provider (matching provider's config)
-		WithRepository(repository1).
-		WithRepository(repository2).
-		WithRepository(repository3).
-		WithRepository(repository4)
-
-	client := newFakeClient(config1).
-		// fake repository for capi, bootstrap and infra provider (matching provider's config)
-		WithRepository(repository1).
-		WithRepository(repository2).
-		WithRepository(repository3).
-		WithRepository(repository4).
-		// fake empty cluster
-		WithCluster(cluster1)
+	return []*fakeRepositoryClient{repository1, repository2, repository3, repository4}
+}
 
+func fakeClusterCtlClient(config *fakeConfigClient, repos []*fakeRepositoryClient, clusters []*fakeClusterClient) *fakeClient {
+	client := newFakeClient(config)
+	for _, r := range repos {
+		client = client.WithRepository(r)
+	}
+	for _, c := range clusters {
+		client = client.WithCluster(c)
+	}
 	return client
 }
 
@@ -549,3 +630,36 @@ func templateYAML(ns string, clusterName string) []byte {
 
 	return podYaml
 }
+
+// infraComponentsYAML defines a namespace and deployment with container
+// images and a variable
+func infraComponentsYAML(namespace string) []byte {
+	var infraComponentsYAML string = `---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: %[1]s
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: capa-controller-manager
+  namespace: %[1]s
+spec:
+  template:
+    spec:
+      containers:
+      - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.4.1
+        name: kube-rbac-proxy
+      - image: us.gcr.io/k8s-artifacts-prod/cluster-api-aws/cluster-api-aws-controller:v0.5.3
+        name: manager
+        volumeMounts:
+        - mountPath: /home/.aws
+          name: credentials
+      volumes:
+      - name: credentials
+        secret:
+          secretName: ${SOME_VARIABLE}
+`
+	return []byte(fmt.Sprintf(infraComponentsYAML, namespace))
+}

cmd/clusterctl/client/repository/components.go

@@ -179,7 +179,8 @@ type ComponentsOptions struct {
 	Version           string
 	TargetNamespace   string
 	WatchingNamespace string
-	SkipVariables     bool
+	// Allows for skipping variable replacement in the component YAML
+	SkipVariables bool
 }
 
 // NewComponents returns a new objects embedding a component YAML file