Merge pull request #621 from alexeldeib/ace/hook

🐛 fix webhooks
kubernetes-sigs · May 15, 2020 · db93854 · db93854
2 parents 4b04993 + 9faa72d
commit db93854
Show file tree

Hide file tree

Showing 11 changed files with 32 additions and 18 deletions.
diff --git a/api/v1alpha3/azurecluster_validation.go b/api/v1alpha3/azurecluster_validation.go
@@ -74,7 +74,7 @@ func validateNetworkSpec(networkSpec NetworkSpec, fldPath *field.Path) field.Err
 
 // validateResourceGroup validates a ResourceGroup
 func validateResourceGroup(resourceGroup string, fldPath *field.Path) *field.Error {
-	if success, _ := regexp.Match(resourceGroupRegex, []byte(resourceGroup)); !success {
+	if success, _ := regexp.MatchString(resourceGroupRegex, resourceGroup); !success {
 		return field.Invalid(fldPath, resourceGroup,
 			fmt.Sprintf("resourceGroup doesn't match regex %s", resourceGroupRegex))
 	}

diff --git a/api/v1alpha3/azurecluster_webhook.go b/api/v1alpha3/azurecluster_webhook.go
@@ -32,7 +32,7 @@ func (c *AzureCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-azurecluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azurecluster,versions=v1alpha3,name=validation.azurecluster.infrastructure.cluster.x-k8s.io,sideEffects=None
+// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-azurecluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azureclusters,versions=v1alpha3,name=validation.azurecluster.infrastructure.cluster.x-k8s.io,sideEffects=None
 
 var _ webhook.Validator = &AzureCluster{}
 

diff --git a/api/v1alpha3/azuremachine_default.go b/api/v1alpha3/azuremachine_default.go
@@ -19,6 +19,7 @@ package v1alpha3
 import (
 	"crypto/rand"
 	"crypto/rsa"
+	"encoding/base64"
 
 	"github.com/pkg/errors"
 	"golang.org/x/crypto/ssh"
@@ -37,7 +38,7 @@ func (m *AzureMachine) SetDefaultSSHPublicKey() error {
 		if perr != nil {
 			return errors.Wrap(perr, "Failed to generate public key")
 		}
-		m.Spec.SSHPublicKey = string(ssh.MarshalAuthorizedKey(publicRsaKey))
+		m.Spec.SSHPublicKey = base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey))
 	}
 
 	return nil

diff --git a/api/v1alpha3/azuremachine_validation.go b/api/v1alpha3/azuremachine_validation.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1alpha3
 
 import (
+	"encoding/base64"
+
 	"golang.org/x/crypto/ssh"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
@@ -25,7 +27,13 @@ import (
 func ValidateSSHKey(sshKey string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if _, _, _, _, err := ssh.ParseAuthorizedKey([]byte(sshKey)); err != nil {
+	decoded, err := base64.StdEncoding.DecodeString(sshKey)
+	if err != nil {
+		allErrs = append(allErrs, field.Required(fldPath, "the SSH public key is not properly base64 encoded"))
+		return allErrs
+	}
+
+	if _, _, _, _, err := ssh.ParseAuthorizedKey(decoded); err != nil {
 		allErrs = append(allErrs, field.Required(fldPath, "the SSH public key is not valid"))
 		return allErrs
 	}

diff --git a/api/v1alpha3/azuremachine_validation_test.go b/api/v1alpha3/azuremachine_validation_test.go
@@ -19,6 +19,7 @@ package v1alpha3
 import (
 	"crypto/rand"
 	"crypto/rsa"
+	"encoding/base64"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -61,5 +62,5 @@ func TestAzureMachine_ValidateSSHKey(t *testing.T) {
 func generateSSHPublicKey() string {
 	privateKey, _ := rsa.GenerateKey(rand.Reader, 2048)
 	publicRsaKey, _ := ssh.NewPublicKey(&privateKey.PublicKey)
-	return string(ssh.MarshalAuthorizedKey(publicRsaKey))
+	return base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey))
 }
diff --git a/api/v1alpha3/azuremachine_webhook.go b/api/v1alpha3/azuremachine_webhook.go
@@ -35,7 +35,7 @@ func (m *AzureMachine) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-azuremachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremachine,versions=v1alpha3,name=validation.azuremachine.infrastructure.cluster.x-k8s.io,sideEffects=None
+// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-azuremachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremachines,versions=v1alpha3,name=validation.azuremachine.infrastructure.cluster.x-k8s.io,sideEffects=None
 
 var _ webhook.Validator = &AzureMachine{}
 

diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -53,7 +53,7 @@ webhooks:
     - UPDATE
     - DELETE
     resources:
-    - azurecluster
+    - azureclusters
   sideEffects: None
 - clientConfig:
     caBundle: Cg==
@@ -73,7 +73,7 @@ webhooks:
     - CREATE
     - UPDATE
     resources:
-    - azuremachine
+    - azuremachines
   sideEffects: None
 - clientConfig:
     caBundle: Cg==

diff --git a/config/webhook/webhookcainjection_patch.yaml b/config/webhook/webhookcainjection_patch.yaml
@@ -2,12 +2,12 @@
 # This patch add annotation to admission webhook config and
 # the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
 # uncomment the following lines to enable mutating and validating webhook
-#apiVersion: admissionregistration.k8s.io/v1beta1
-#kind: MutatingWebhookConfiguration
-#metadata:
-#  name: mutating-webhook-configuration
-#  annotations:
-#    cert-manager.k8s.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: MutatingWebhookConfiguration
+metadata:
+ name: mutating-webhook-configuration
+ annotations:
+   cert-manager.k8s.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
 ---
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration

diff --git a/test/e2e/azure_suite_test.go b/test/e2e/azure_suite_test.go
@@ -128,6 +128,7 @@ var _ = BeforeSuite(func() {
 	framework.InstallComponents(ctx, mgmt, capi, cabpk, kcp, infra)
 	framework.WaitForPodsReadyInNamespace(ctx, mgmt, "capi-system")
 	framework.WaitForPodsReadyInNamespace(ctx, mgmt, "capz-system")
+	framework.WaitForPodsReadyInNamespace(ctx, mgmt, "capi-webhook-system")
 
 	// go func() {
 	// 	defer GinkgoRecover()
@@ -143,6 +144,7 @@ var _ = AfterSuite(func() {
 	// DO NOT stream "capi-controller-manager" logs as it prints out azure.json
 	Expect(writeLogs(mgmt, "capi-kubeadm-bootstrap-system", "capi-kubeadm-bootstrap-controller-manager", logPath)).To(Succeed())
 	Expect(writeLogs(mgmt, "capz-system", "capz-controller-manager", logPath)).To(Succeed())
+	Expect(writeLogs(mgmt, "capi-webhook-system", "capz-controller-manager", logPath)).To(Succeed())
 	By("Tearing down management cluster")
 	mgmt.Teardown(ctx)
 })

diff --git a/test/e2e/azure_test.go b/test/e2e/azure_test.go
@@ -51,7 +51,6 @@ var _ = Describe("CAPZ e2e tests", func() {
 			nodeGen = &NodeGenerator{}
 			clusterGen.VariablesInit()
 			machineDeploymentGen = &MachineDeploymentGenerator{}
-			cluster, infraCluster = clusterGen.GenerateCluster(namespace)
 		})
 
 		AfterEach(func() {
@@ -61,6 +60,7 @@ var _ = Describe("CAPZ e2e tests", func() {
 
 		Context("Create single controlplane cluster", func() {
 			It("Should create a single node cluster", func() {
+				cluster, infraCluster = clusterGen.GenerateCluster(creds.SubscriptionID, namespace)
 				controlplane := nodeGen.GenerateKubeadmControlplane(creds, cluster.GetName(), 1)
 				machineTemplate := nodeGen.GenerateMachineTemplate(creds, cluster.GetName())
 				input = &ControlPlaneClusterInput{
@@ -77,6 +77,7 @@ var _ = Describe("CAPZ e2e tests", func() {
 
 		Context("Create multiple controlplane cluster with machine deployments", func() {
 			It("Should create a 3 node cluster", func() {
+				cluster, infraCluster = clusterGen.GenerateCluster(creds.SubscriptionID, namespace)
 				controlplane := nodeGen.GenerateKubeadmControlplane(creds, cluster.GetName(), 3)
 				machineTemplate := nodeGen.GenerateMachineTemplate(creds, cluster.GetName())
 				machineDeployment := machineDeploymentGen.Generate(creds, cluster.GetNamespace(), cluster.GetName(), 1)

diff --git a/test/e2e/resource_generators.go b/test/e2e/resource_generators.go
@@ -117,7 +117,7 @@ func (c *ClusterGenerator) VariablesInit() {
 	imageVersion = v.GetString("imageVersion")
 }
 
-func (c *ClusterGenerator) GenerateCluster(namespace string) (*capiv1.Cluster, *infrav1.AzureCluster) {
+func (c *ClusterGenerator) GenerateCluster(subscriptionId, namespace string) (*capiv1.Cluster, *infrav1.AzureCluster) {
 	name := "capz-e2e" + util.RandomString(6)
 	vnetName := name + "-vnet"
 	tags := map[string]string{
@@ -130,8 +130,9 @@ func (c *ClusterGenerator) GenerateCluster(namespace string) (*capiv1.Cluster, *
 			Name:      name,
 		},
 		Spec: infrav1.AzureClusterSpec{
-			Location:      location,
-			ResourceGroup: name,
+			Location:       location,
+			ResourceGroup:  name,
+			SubscriptionID: subscriptionId,
 			NetworkSpec: infrav1.NetworkSpec{
 				Vnet: infrav1.VnetSpec{Name: vnetName},
 			},