Improve clusterctl upgrade syntax. Don't require namespace

cmd/clusterctl/client/client.go

@@ -125,7 +125,7 @@ type RepositoryClientFactoryInput struct {
 // RepositoryClientFactory is a factory of repository.Client from a given input.
 type RepositoryClientFactory func(RepositoryClientFactoryInput) (repository.Client, error)
 
-// ClusterClientFactoryInput reporesents the inputs required by the factory.
+// ClusterClientFactoryInput represents the inputs required by the factory.
 type ClusterClientFactoryInput struct {
 	Kubeconfig Kubeconfig
 	Processor  Processor

cmd/clusterctl/client/clusterclass.go

@@ -60,12 +60,12 @@ func addClusterClassIfMissing(template Template, clusterClassClient repository.C
 }
 
 // clusterClassNamesFromTemplate returns the list of ClusterClasses referenced
-// by custers defined in the template. If not clusters are defined in the template
+// by clusters defined in the template. If not clusters are defined in the template
 // or if no cluster uses a cluster class it returns an empty list.
 func clusterClassNamesFromTemplate(template Template) ([]string, error) {
 	classes := []string{}
 
-	// loop thorugh all the objects and if the object is a cluster
+	// loop through all the objects and if the object is a cluster
 	// check and see if cluster.spec.topology.class is defined.
 	// If defined, add value to the result.
 	for i := range template.Objs() {

cmd/clusterctl/client/upgrade.go

@@ -176,28 +176,28 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error {
 		upgradeItems := []cluster.UpgradeItem{}
 
 		if options.CoreProvider != "" {
-			upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.CoreProviderType, options.CoreProvider)
+			upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.CoreProviderType, options.CoreProvider)
 			if err != nil {
 				return err
 			}
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.BootstrapProviderType, options.BootstrapProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.BootstrapProviderType, options.BootstrapProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.ControlPlaneProviderType, options.ControlPlaneProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.ControlPlaneProviderType, options.ControlPlaneProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.InfrastructureProviderType, options.InfrastructureProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.InfrastructureProviderType, options.InfrastructureProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.IPAMProviderType, options.IPAMProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.IPAMProviderType, options.IPAMProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.RuntimeExtensionProviderType, options.RuntimeExtensionProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.RuntimeExtensionProviderType, options.RuntimeExtensionProviders...)
 		if err != nil {
 			return err
 		}
@@ -210,9 +210,9 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error {
 	return clusterClient.ProviderUpgrader().ApplyPlan(opts, options.Contract)
 }
 
-func addUpgradeItems(upgradeItems []cluster.UpgradeItem, providerType clusterctlv1.ProviderType, providers ...string) ([]cluster.UpgradeItem, error) {
+func addUpgradeItems(clusterClient cluster.Client, upgradeItems []cluster.UpgradeItem, providerType clusterctlv1.ProviderType, providers ...string) ([]cluster.UpgradeItem, error) {
 	for _, upgradeReference := range providers {
-		providerUpgradeItem, err := parseUpgradeItem(upgradeReference, providerType)
+		providerUpgradeItem, err := parseUpgradeItem(clusterClient, upgradeReference, providerType)
 		if err != nil {
 			return nil, err
 		}
@@ -224,10 +224,22 @@ func addUpgradeItems(upgradeItems []cluster.UpgradeItem, providerType clusterctl
 	return upgradeItems, nil
 }
 
-func parseUpgradeItem(ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+func parseUpgradeItem(clusterClient cluster.Client, ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+	// TODO(oscr) Remove when explicit namespaces for providers is removed
+	// ref format is old format: namespace/provider[:version]
+	if strings.Contains(ref, "/") {
+		return parseUpgradeItemWithNamespace(clusterClient, ref, providerType)
+	}
+
+	// ref format is: provider:version
+	return parseUpgradeItemWithoutNamespace(clusterClient, ref, providerType)
+}
+
+func parseUpgradeItemWithNamespace(clusterClient cluster.Client, ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
 	refSplit := strings.Split(strings.ToLower(ref), "/")
+
 	if len(refSplit) != 2 {
-		return nil, errors.Errorf("invalid provider name %q. Provider name should be in the form namespace/provider[:version]", ref)
+		return nil, errors.Errorf("invalid provider name %q. Provider name should be in the form namespace/provider[:version]  or provider:version", ref)
 	}
 
 	if refSplit[0] == "" {
@@ -255,3 +267,34 @@ func parseUpgradeItem(ref string, providerType clusterctlv1.ProviderType) (*clus
 		NextVersion: version,
 	}, nil
 }
+
+func parseUpgradeItemWithoutNamespace(clusterClient cluster.Client, ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+	if !strings.Contains(ref, ":") {
+		return nil, errors.Errorf("invalid provider name %q. Provider name should be in the form namespace/name[:version] and namespace cannot be empty", ref)
+	}
+
+	name, version, err := parseProviderName(ref)
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid provider name %q. Provider name should be in the form namespace/name[:version] and the namespace should be valid", ref)
+	}
+
+	namespace, err := clusterClient.ProviderInventory().GetProviderNamespace(name, providerType)
+	if err != nil {
+		return nil, errors.Errorf("unable to find default namespace for provider %q", ref)
+	}
+
+	return &cluster.UpgradeItem{
+		Provider: clusterctlv1.Provider{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: namespace,
+				Name:      clusterctlv1.ManifestLabel(name, providerType),
+			},
+			ProviderName: name,
+			Type:         string(providerType),
+			// The value for the following fields will be retrieved while
+			// creating the custom upgrade plan.
+			WatchedNamespace: "",
+		},
+		NextVersion: version,
+	}, nil
+}

cmd/clusterctl/client/upgrade_test.go

@@ -375,6 +375,11 @@ func Test_parseUpgradeItem(t *testing.T) {
 	type args struct {
 		provider string
 	}
+
+	configClient := newFakeConfig()
+	clusterClient := newFakeCluster(cluster.Kubeconfig{Path: "cluster1"}, configClient)
+	clusterClient.WithProviderInventory("best-provider", clusterctlv1.CoreProviderType, "v1.0.0", "best-provider-system")
+
 	tests := []struct {
 		name    string
 		args    args
@@ -418,9 +423,35 @@ func Test_parseUpgradeItem(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "namespace missing",
+			name: "provider:version",
+			args: args{
+				provider: "best-provider:v1.0.0",
+			},
+			want: &cluster.UpgradeItem{
+				Provider: clusterctlv1.Provider{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "best-provider-system",
+						Name:      clusterctlv1.ManifestLabel("best-provider", clusterctlv1.CoreProviderType),
+					},
+					ProviderName: "best-provider",
+					Type:         string(clusterctlv1.CoreProviderType),
+				},
+				NextVersion: "v1.0.0",
+			},
+			wantErr: false,
+		},
+		{
+			name: "provider: with no version",
+			args: args{
+				provider: "provider:",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "provider with no version",
 			args: args{
-				provider: "provider:version",
+				provider: "provider",
 			},
 			want:    nil,
 			wantErr: true,
@@ -438,7 +469,7 @@ func Test_parseUpgradeItem(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			got, err := parseUpgradeItem(tt.args.provider, clusterctlv1.CoreProviderType)
+			got, err := parseUpgradeItem(clusterClient, tt.args.provider, clusterctlv1.CoreProviderType)
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 				return