Merge pull request #4677 from willie-yao/fix-extension

k8s-ci-robot · web-flow · commit d0f78e009bb1 · 2024-03-27T13:54:26.000-07:00
Make AKS extension.Plan optional
diff --git a/api/v1beta1/azuremanagedcontrolplane_default.go b/api/v1beta1/azuremanagedcontrolplane_default.go
@@ -210,7 +210,7 @@ func (m *AzureManagedControlPlane) setDefaultDNSPrefix() {
 
 func (m *AzureManagedControlPlane) setDefaultAKSExtensions() {
 	for _, extension := range m.Spec.Extensions {
-		if extension.Plan.Name == "" {
+		if extension.Plan != nil && extension.Plan.Name == "" {
 			extension.Plan.Name = fmt.Sprintf("%s-%s", m.Name, extension.Plan.Product)
 		}
 		if extension.AutoUpgradeMinorVersion == nil {
diff --git a/api/v1beta1/azuremanagedcontrolplane_types.go b/api/v1beta1/azuremanagedcontrolplane_types.go
@@ -608,7 +608,8 @@ type AKSExtension struct {
 	ExtensionType *string `json:"extensionType"`
 
 	// Plan is the plan of the extension.
-	Plan *ExtensionPlan `json:"plan"`
+	// +optional
+	Plan *ExtensionPlan `json:"plan,omitempty"`
 
 	// ReleaseTrain is the release train this extension participates in for auto-upgrade (e.g. Stable, Preview, etc.)
 	// This is only used if autoUpgradeMinorVersion is ‘true’.
diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go
@@ -1002,12 +1002,6 @@ func validateAKSExtensions(extensions []AKSExtension, fldPath *field.Path) field
 		if extension.Version != nil && (extension.AutoUpgradeMinorVersion == nil || (extension.AutoUpgradeMinorVersion != nil && *extension.AutoUpgradeMinorVersion)) {
 			allErrs = append(allErrs, field.Forbidden(fldPath.Child("Version"), "Version must not be given if AutoUpgradeMinorVersion is true (or not provided, as it is true by default)"))
 		}
-		if extension.Plan.Product == "" {
-			allErrs = append(allErrs, field.Required(fldPath.Child("Plan", "Product"), "Product must be provided"))
-		}
-		if extension.Plan.Publisher == "" {
-			allErrs = append(allErrs, field.Required(fldPath.Child("Plan", "Publisher"), "Publisher must be provided"))
-		}
 		if extension.AutoUpgradeMinorVersion == ptr.To(false) && extension.ReleaseTrain != nil {
 			allErrs = append(allErrs, field.Forbidden(fldPath.Child("ReleaseTrain"), "ReleaseTrain must not be given if AutoUpgradeMinorVersion is false"))
 		}
diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go
@@ -1119,14 +1119,16 @@ type ExtensionPlan struct {
 	Name string `json:"name,omitempty"`
 
 	// Product is the name of the 3rd Party artifact that is being procured.
-	Product string `json:"product"`
+	// +optional
+	Product string `json:"product,omitempty"`
 
 	// PromotionCode is a publisher-provided promotion code as provisioned in Data Market for the said product/artifact.
 	// +optional
 	PromotionCode string `json:"promotionCode,omitempty"`
 
 	// Publisher is the name of the publisher of the 3rd Party Artifact that is being bought.
-	Publisher string `json:"publisher"`
+	// +optional
+	Publisher string `json:"publisher,omitempty"`
 
 	// Version is the version of the plan.
 	// +optional
diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go
@@ -1030,7 +1030,7 @@ func (s *ManagedControlPlaneScope) AKSExtensionSpecs() []azure.ASOResourceSpecGe
 			ReleaseTrain:            extension.ReleaseTrain,
 			Version:                 extension.Version,
 			Owner:                   azure.ManagedClusterID(s.SubscriptionID(), s.ResourceGroup(), s.ControlPlane.Name),
-			Plan:                    *extension.Plan,
+			Plan:                    extension.Plan,
 			AKSAssignedIdentityType: extension.AKSAssignedIdentityType,
 			ExtensionIdentity:       extension.Identity,
 		}
diff --git a/azure/scope/managedcontrolplane_test.go b/azure/scope/managedcontrolplane_test.go
@@ -1510,7 +1510,7 @@ func TestManagedControlPlaneScope_AKSExtensionSpecs(t *testing.T) {
 					ReleaseTrain:  ptr.To("my-release-train"),
 					Version:       ptr.To("my-version"),
 					Owner:         "/subscriptions//resourceGroups//providers/Microsoft.ContainerService/managedClusters/my-cluster",
-					Plan: infrav1.ExtensionPlan{
+					Plan: &infrav1.ExtensionPlan{
 						Name:      "my-plan-name",
 						Product:   "my-product",
 						Publisher: "my-publisher",
diff --git a/azure/services/aksextensions/spec.go b/azure/services/aksextensions/spec.go
@@ -39,7 +39,7 @@ type AKSExtensionSpec struct {
 	Version                 *string
 	Owner                   string
 	OwnerRef                metav1.OwnerReference
-	Plan                    infrav1.ExtensionPlan
+	Plan                    *infrav1.ExtensionPlan
 	Scope                   infrav1.ExtensionScope
 }
 
@@ -70,11 +70,14 @@ func (s *AKSExtensionSpec) Parameters(ctx context.Context, existingAKSExtension
 	aksExtension.Spec.Owner = &genruntime.ArbitraryOwnerReference{
 		ARMID: s.Owner,
 	}
-	aksExtension.Spec.Plan = &asokubernetesconfigurationv1.Plan{
-		Name:      ptr.To(s.Plan.Name),
-		Product:   ptr.To(s.Plan.Product),
-		Publisher: ptr.To(s.Plan.Publisher),
-		Version:   ptr.To(s.Plan.Version),
+
+	if s.Plan != nil {
+		aksExtension.Spec.Plan = &asokubernetesconfigurationv1.Plan{
+			Name:      ptr.To(s.Plan.Name),
+			Product:   ptr.To(s.Plan.Product),
+			Publisher: ptr.To(s.Plan.Publisher),
+			Version:   ptr.To(s.Plan.Version),
+		}
 	}
 	if s.ExtensionIdentity != "" {
 		aksExtension.Spec.Identity = &asokubernetesconfigurationv1.Identity{
diff --git a/azure/services/aksextensions/spec_test.go b/azure/services/aksextensions/spec_test.go
@@ -68,7 +68,7 @@ var (
 		ReleaseTrain:  ptr.To("fake-release-train"),
 		Version:       ptr.To("fake-version"),
 		Owner:         "fake-owner",
-		Plan: infrav1.ExtensionPlan{
+		Plan: &infrav1.ExtensionPlan{
 			Name: "fake-plan-name",
 		},
 		ExtensionIdentity: "SystemAssigned",
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml
@@ -367,9 +367,6 @@ spec:
                         version:
                           description: Version is the version of the plan.
                           type: string
-                      required:
-                      - product
-                      - publisher
                       type: object
                     releaseTrain:
                       description: ReleaseTrain is the release train this extension
@@ -405,7 +402,6 @@ spec:
                   required:
                   - extensionType
                   - name
-                  - plan
                   type: object
                 type: array
               fleetsMember:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml
@@ -351,9 +351,6 @@ spec:
                                 version:
                                   description: Version is the version of the plan.
                                   type: string
-                              required:
-                              - product
-                              - publisher
                               type: object
                             releaseTrain:
                               description: ReleaseTrain is the release train this
@@ -392,7 +389,6 @@ spec:
                           required:
                           - extensionType
                           - name
-                          - plan
                           type: object
                         type: array
                       fleetsMember:
diff --git a/docs/book/src/topics/managedcluster.md b/docs/book/src/topics/managedcluster.md
@@ -326,7 +326,13 @@ spec:
       publisher: "containous"
 ```
 
-To find the `extensionType` and plan details for your desired extension, refer to the [az k8s-extension cli reference](https://learn.microsoft.com/cli/azure/k8s-extension).
+To list all of the available extensions for your cluster as well as its plan details, use the following az cli command:
+
+```bash
+az k8s-extension extension-types list-by-cluster --resource-group my-resource-group --cluster-name mycluster --cluster-type managedClusters
+```
+
+For more details, please refer to the [az k8s-extension cli reference](https://learn.microsoft.com/cli/azure/k8s-extension).
 
 
 ### Security Profile for AKS clusters.
diff --git a/test/e2e/aks_marketplace.go b/test/e2e/aks_marketplace.go
@@ -43,7 +43,8 @@ type AKSMarketplaceExtensionSpecInput struct {
 }
 
 const (
-	extensionName = "aks-marketplace-extension"
+	extensionName         = "aks-marketplace-extension"
+	officialExtensionName = "official-aks-extension"
 )
 
 func AKSMarketplaceExtensionSpec(ctx context.Context, inputGetter func() AKSMarketplaceExtensionSpecInput) {
@@ -127,7 +128,7 @@ func AKSMarketplaceExtensionSpec(ctx context.Context, inputGetter func() AKSMark
 	}, input.WaitIntervals...).Should(Succeed())
 	Eventually(checkTaints, input.WaitIntervals...).Should(Succeed())
 
-	By("Adding an AKS Marketplace Extension to the AzureManagedControlPlane")
+	By("Adding an official AKS Extension & AKS Marketplace Extension to the AzureManagedControlPlane")
 	var infraControlPlane = &infrav1.AzureManagedControlPlane{}
 	Eventually(func(g Gomega) {
 		err = mgmtClient.Get(ctx, client.ObjectKey{
@@ -145,6 +146,10 @@ func AKSMarketplaceExtensionSpec(ctx context.Context, inputGetter func() AKSMark
 					Publisher: "containous",
 				},
 			},
+			{
+				Name:          officialExtensionName,
+				ExtensionType: ptr.To("microsoft.flux"),
+			},
 		}
 		g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed())
 	}, input.WaitIntervals...).Should(Succeed())
@@ -157,16 +162,8 @@ func AKSMarketplaceExtensionSpec(ctx context.Context, inputGetter func() AKSMark
 	}, input.WaitIntervals...).Should(Succeed())
 
 	By("Ensuring the AKS Marketplace Extension is added to the AzureManagedControlPlane")
-	Eventually(func(g Gomega) {
-		resp, err := extensionClient.Get(ctx, amcp.Spec.ResourceGroupName, "Microsoft.ContainerService", "managedClusters", input.Cluster.Name, extensionName, nil)
-		g.Expect(err).NotTo(HaveOccurred())
-		g.Expect(resp.Properties.ProvisioningState).To(Equal(ptr.To(armkubernetesconfiguration.ProvisioningStateSucceeded)))
-		extension := resp.Extension
-		g.Expect(extension.Properties).NotTo(BeNil())
-		g.Expect(extension.Name).To(Equal(ptr.To(extensionName)))
-		g.Expect(extension.Properties.AutoUpgradeMinorVersion).To(Equal(ptr.To(true)))
-		g.Expect(extension.Properties.ExtensionType).To(Equal(ptr.To("TraefikLabs.TraefikProxy")))
-	}, input.WaitIntervals...).Should(Succeed())
+	ensureAKSExtensionAdded(ctx, input, extensionName, "TraefikLabs.TraefikProxy", extensionClient, amcp)
+	ensureAKSExtensionAdded(ctx, input, officialExtensionName, "microsoft.flux", extensionClient, amcp)
 
 	By("Restoring initial taints for Windows machine pool")
 	expectedTaints = initialTaints
@@ -177,3 +174,16 @@ func AKSMarketplaceExtensionSpec(ctx context.Context, inputGetter func() AKSMark
 	}, input.WaitIntervals...).Should(Succeed())
 	Eventually(checkTaints, input.WaitIntervals...).Should(Succeed())
 }
+
+func ensureAKSExtensionAdded(ctx context.Context, input AKSMarketplaceExtensionSpecInput, extensionName, extensionType string, extensionClient *armkubernetesconfiguration.ExtensionsClient, amcp *infrav1.AzureManagedControlPlane) {
+	Eventually(func(g Gomega) {
+		resp, err := extensionClient.Get(ctx, amcp.Spec.ResourceGroupName, "Microsoft.ContainerService", "managedClusters", input.Cluster.Name, extensionName, nil)
+		g.Expect(err).NotTo(HaveOccurred())
+		g.Expect(resp.Properties.ProvisioningState).To(Equal(ptr.To(armkubernetesconfiguration.ProvisioningStateSucceeded)))
+		extension := resp.Extension
+		g.Expect(extension.Properties).NotTo(BeNil())
+		g.Expect(extension.Name).To(Equal(ptr.To(extensionName)))
+		g.Expect(extension.Properties.AutoUpgradeMinorVersion).To(Equal(ptr.To(true)))
+		g.Expect(extension.Properties.ExtensionType).To(Equal(ptr.To(extensionType)))
+	}, input.WaitIntervals...).Should(Succeed())
+}

Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ func (m *AzureManagedControlPlane) setDefaultDNSPrefix() {`
`210`	`210`
`211`	`211`	`func (m *AzureManagedControlPlane) setDefaultAKSExtensions() {`
`212`	`212`	`for _, extension := range m.Spec.Extensions {`
`213`		`- if extension.Plan.Name == "" {`
	`213`	`+ if extension.Plan != nil && extension.Plan.Name == "" {`
`214`	`214`	`extension.Plan.Name = fmt.Sprintf("%s-%s", m.Name, extension.Plan.Product)`
`215`	`215`	`}`
`216`	`216`	`if extension.AutoUpgradeMinorVersion == nil {`
Original file line number	Diff line number	Diff line change
`@@ -1002,12 +1002,6 @@ func validateAKSExtensions(extensions []AKSExtension, fldPath *field.Path) field`
`1002`	`1002`	`if extension.Version != nil && (extension.AutoUpgradeMinorVersion == nil \|\| (extension.AutoUpgradeMinorVersion != nil && *extension.AutoUpgradeMinorVersion)) {`
`1003`	`1003`	`allErrs = append(allErrs, field.Forbidden(fldPath.Child("Version"), "Version must not be given if AutoUpgradeMinorVersion is true (or not provided, as it is true by default)"))`
`1004`	`1004`	`}`
`1005`		`- if extension.Plan.Product == "" {`
`1006`		`- allErrs = append(allErrs, field.Required(fldPath.Child("Plan", "Product"), "Product must be provided"))`
`1007`		`- }`
`1008`		`- if extension.Plan.Publisher == "" {`
`1009`		`- allErrs = append(allErrs, field.Required(fldPath.Child("Plan", "Publisher"), "Publisher must be provided"))`
`1010`		`- }`
`1011`	`1005`	`if extension.AutoUpgradeMinorVersion == ptr.To(false) && extension.ReleaseTrain != nil {`
`1012`	`1006`	`allErrs = append(allErrs, field.Forbidden(fldPath.Child("ReleaseTrain"), "ReleaseTrain must not be given if AutoUpgradeMinorVersion is false"))`
`1013`	`1007`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1030,7 +1030,7 @@ func (s *ManagedControlPlaneScope) AKSExtensionSpecs() []azure.ASOResourceSpecGe`
`1030`	`1030`	`ReleaseTrain: extension.ReleaseTrain,`
`1031`	`1031`	`Version: extension.Version,`
`1032`	`1032`	`Owner: azure.ManagedClusterID(s.SubscriptionID(), s.ResourceGroup(), s.ControlPlane.Name),`
`1033`		`- Plan: *extension.Plan,`
	`1033`	`+ Plan: extension.Plan,`
`1034`	`1034`	`AKSAssignedIdentityType: extension.AKSAssignedIdentityType,`
`1035`	`1035`	`ExtensionIdentity: extension.Identity,`
`1036`	`1036`	`}`