diff --git a/cmd/machineset/main.go b/cmd/machineset/main.go index 91d3d57546..103a048ff4 100644 --- a/cmd/machineset/main.go +++ b/cmd/machineset/main.go @@ -80,21 +80,37 @@ func main() { } // Enable defaulting and validating webhooks - defaulter, err := v1beta1.NewMachineDefaulter() + machineDefaulter, err := v1beta1.NewMachineDefaulter() if err != nil { log.Fatal(err) } - validator, err := v1beta1.NewMachineValidator() + machineValidator, err := v1beta1.NewMachineValidator() if err != nil { log.Fatal(err) } + machineSetDefaulter, err := v1beta1.NewMachineSetDefaulter() + if err != nil { + log.Fatal(err) + } + + machineSetValidator, err := v1beta1.NewMachineSetValidator() + if err != nil { + log.Fatal(err) + } + + machineSetCPValidator := v1beta1.NewMachineSetCPValidator() + if *webhookEnabled { mgr.GetWebhookServer().Port = *webhookPort mgr.GetWebhookServer().CertDir = *webhookCertdir - mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: defaulter}) - mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: validator}) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineValidator}) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetValidator}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: machineSetCPValidator}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: machineSetCPValidator}) } log.Printf("Registering Components.") diff --git a/go.mod b/go.mod index 453c8ac0f7..762bf13567 100644 --- a/go.mod +++ b/go.mod @@ -39,3 +39,5 @@ replace sigs.k8s.io/cluster-api-provider-aws => github.com/openshift/cluster-api replace sigs.k8s.io/cluster-api-provider-azure => github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20200529030741-17d4edc5142f replace sigs.k8s.io/cluster-api-provider-gcp => github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200528175251-4f2fdeb49fe1 + +replace sigs.k8s.io/controller-runtime => github.com/alvaroaleman/controller-runtime v0.1.5-0.20200619152754-4a802fb9b747 diff --git a/go.sum b/go.sum index 974a3f1de7..ebbc07efcd 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,8 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuy github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/alvaroaleman/controller-runtime v0.1.5-0.20200619152754-4a802fb9b747 h1:l5sFFGjYd9YsVYN8u1JRsa4pSURnxBEpM80B1W2y46E= +github.com/alvaroaleman/controller-runtime v0.1.5-0.20200619152754-4a802fb9b747/go.mod h1:qN/IYzFHXI7mP9qhUiGRN9uDH3fdAAqBTCqP1YkMEtQ= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= diff --git a/install/0000_30_machine-api-operator_08_webhook.yaml b/install/0000_30_machine-api-operator_08_webhook.yaml index 2f71c2fb85..e8caf1b0ad 100644 --- a/install/0000_30_machine-api-operator_08_webhook.yaml +++ b/install/0000_30_machine-api-operator_08_webhook.yaml @@ -28,6 +28,26 @@ webhooks: resources: - machines sideEffects: None + - clientConfig: + service: + name: machine-api-operator-webhook + namespace: openshift-machine-api + path: /mutate-machine-openshift-io-v1beta1-machineset + # failurePolicy is ignore so we don't want to block machine lifecycle on the webhook operational aspects. + # This would be particularly problematic for chicken egg issues when bootstrapping a cluster. + failurePolicy: Ignore + matchPolicy: Equivalent + name: default.machineset.machine.openshift.io + rules: + - apiGroups: + - machine.openshift.io + apiVersions: + - v1beta1 + operations: + - CREATE + resources: + - machinesets + sideEffects: None --- apiVersion: admissionregistration.k8s.io/v1beta1 kind: ValidatingWebhookConfiguration @@ -59,3 +79,64 @@ webhooks: resources: - machines sideEffects: None + - clientConfig: + service: + name: machine-api-operator-webhook + namespace: openshift-machine-api + path: /validate-machine-openshift-io-v1beta1-machineset + # failurePolicy is ignore so we don't want to block machine lifecycle on the webhook operational aspects. + # This would be particularly problematic for chicken egg issues when bootstrapping a cluster. + failurePolicy: Ignore + matchPolicy: Equivalent + name: validation.machineset.machine.openshift.io + rules: + - apiGroups: + - machine.openshift.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - machinesets + sideEffects: None + - clientConfig: + service: + name: machine-api-operator-webhook + namespace: openshift-machine-api + path: /validate-machine-openshift-io-v1beta1-machineset-cp-delete + # failurePolicy is Fail so we ensure control plane machinesets are never + # deleted. + failurePolicy: Fail + matchPolicy: Equivalent + name: delete.cp.validation.machineset.machine.openshift.io + rules: + - apiGroups: + - machine.openshift.io + apiVersions: + - v1beta1 + operations: + - DELETE + resources: + - machinesets + sideEffects: None + - clientConfig: + service: + name: machine-api-operator-webhook + namespace: openshift-machine-api + path: /validate-machine-openshift-io-v1beta1-machineset-cp-update + # failurePolicy is Fail so we ensure control plane machinesets are never + # made non-CP machinesets. + failurePolicy: Fail + matchPolicy: Equivalent + name: update.cp.validation.machineset.machine.openshift.io + rules: + - apiGroups: + - machine.openshift.io + apiVersions: + - v1beta1 + operations: + - UPDATE + resources: + - machinesets + sideEffects: None diff --git a/pkg/apis/machine/v1beta1/machine_webhook.go b/pkg/apis/machine/v1beta1/machine_webhook.go index 7edc742bb6..8fa8595f9a 100644 --- a/pkg/apis/machine/v1beta1/machine_webhook.go +++ b/pkg/apis/machine/v1beta1/machine_webhook.go @@ -119,29 +119,36 @@ func getInfra() (*osconfigv1.Infrastructure, error) { return infra, nil } -type handlerValidationFn func(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) -type handlerMutationFn func(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) +type machineAdmissionFn func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) -// validatorHandler validates Machine API resources. -// implements type Handler interface. -// https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler -type validatorHandler struct { +type admissionHandler struct { clusterID string - webhookOperations handlerValidationFn + webhookOperations machineAdmissionFn decoder *admission.Decoder } -// defaulterHandler defaults Machine API resources. +// InjectDecoder injects the decoder. +func (a *admissionHandler) InjectDecoder(d *admission.Decoder) error { + a.decoder = d + return nil +} + +// machineValidatorHandler validates Machine API resources. // implements type Handler interface. // https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler -type defaulterHandler struct { - clusterID string - webhookOperations handlerMutationFn - decoder *admission.Decoder +type machineValidatorHandler struct { + *admissionHandler +} + +// machineDefaulterHandler defaults Machine API resources. +// implements type Handler interface. +// https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler +type machineDefaulterHandler struct { + *admissionHandler } -// NewValidator returns a new validatorHandler. -func NewMachineValidator() (*validatorHandler, error) { +// NewValidator returns a new machineValidatorHandler. +func NewMachineValidator() (*machineValidatorHandler, error) { infra, err := getInfra() if err != nil { return nil, err @@ -150,29 +157,33 @@ func NewMachineValidator() (*validatorHandler, error) { return createMachineValidator(infra.Status.PlatformStatus.Type, infra.Status.InfrastructureName), nil } -func createMachineValidator(platform osconfigv1.PlatformType, clusterID string) *validatorHandler { - h := &validatorHandler{ - clusterID: clusterID, +func createMachineValidator(platform osconfigv1.PlatformType, clusterID string) *machineValidatorHandler { + return &machineValidatorHandler{ + admissionHandler: &admissionHandler{ + clusterID: clusterID, + webhookOperations: getMachineValidatorOperation(platform), + }, } +} +func getMachineValidatorOperation(platform osconfigv1.PlatformType) machineAdmissionFn { switch platform { case osconfigv1.AWSPlatformType: - h.webhookOperations = validateAWS + return validateAWS case osconfigv1.AzurePlatformType: - h.webhookOperations = validateAzure + return validateAzure case osconfigv1.GCPPlatformType: - h.webhookOperations = validateGCP + return validateGCP default: // just no-op - h.webhookOperations = func(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) { + return func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { return true, nil } } - return h } -// NewDefaulter returns a new defaulterHandler. -func NewMachineDefaulter() (*defaulterHandler, error) { +// NewDefaulter returns a new machineDefaulterHandler. +func NewMachineDefaulter() (*machineDefaulterHandler, error) { infra, err := getInfra() if err != nil { return nil, err @@ -181,41 +192,37 @@ func NewMachineDefaulter() (*defaulterHandler, error) { return createMachineDefaulter(infra.Status.PlatformStatus, infra.Status.InfrastructureName), nil } -func createMachineDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID string) *defaulterHandler { - h := &defaulterHandler{ - clusterID: clusterID, +func createMachineDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID string) *machineDefaulterHandler { + return &machineDefaulterHandler{ + admissionHandler: &admissionHandler{ + clusterID: clusterID, + webhookOperations: getMachineDefaulterOperation(platformStatus), + }, } +} +func getMachineDefaulterOperation(platformStatus *osconfigv1.PlatformStatus) machineAdmissionFn { switch platformStatus.Type { case osconfigv1.AWSPlatformType: - h.webhookOperations = defaultAWS + return defaultAWS case osconfigv1.AzurePlatformType: - h.webhookOperations = defaultAzure + return defaultAzure case osconfigv1.GCPPlatformType: - h.webhookOperations = gcpDefaulter{projectID: platformStatus.GCP.ProjectID}.defaultGCP + projectID := "" + if platformStatus.GCP != nil { + projectID = platformStatus.GCP.ProjectID + } + return gcpDefaulter{projectID: projectID}.defaultGCP default: // just no-op - h.webhookOperations = func(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { + return func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { return true, nil } } - return h -} - -// InjectDecoder injects the decoder. -func (v *validatorHandler) InjectDecoder(d *admission.Decoder) error { - v.decoder = d - return nil -} - -// InjectDecoder injects the decoder. -func (v *defaulterHandler) InjectDecoder(d *admission.Decoder) error { - v.decoder = d - return nil } // Handle handles HTTP requests for admission webhook servers. -func (h *validatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response { +func (h *machineValidatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response { m := &Machine{} if err := h.decoder.Decode(req, m); err != nil { @@ -224,7 +231,7 @@ func (h *validatorHandler) Handle(ctx context.Context, req admission.Request) ad klog.V(3).Infof("Validate webhook called for Machine: %s", m.GetName()) - if ok, err := h.webhookOperations(h, m); !ok { + if ok, err := h.webhookOperations(m, h.clusterID); !ok { return admission.Denied(err.Error()) } @@ -232,7 +239,7 @@ func (h *validatorHandler) Handle(ctx context.Context, req admission.Request) ad } // Handle handles HTTP requests for admission webhook servers. -func (h *defaulterHandler) Handle(ctx context.Context, req admission.Request) admission.Response { +func (h *machineDefaulterHandler) Handle(ctx context.Context, req admission.Request) admission.Response { m := &Machine{} if err := h.decoder.Decode(req, m); err != nil { @@ -241,7 +248,7 @@ func (h *defaulterHandler) Handle(ctx context.Context, req admission.Request) ad klog.V(3).Infof("Mutate webhook called for Machine: %s", m.GetName()) - if ok, err := h.webhookOperations(h, m); !ok { + if ok, err := h.webhookOperations(m, h.clusterID); !ok { return admission.Denied(err.Error()) } @@ -252,7 +259,7 @@ func (h *defaulterHandler) Handle(ctx context.Context, req admission.Request) ad return admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachine) } -func defaultAWS(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { +func defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting AWS providerSpec") var errs []error @@ -266,7 +273,7 @@ func defaultAWS(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { providerSpec.InstanceType = defaultAWSInstanceType } if providerSpec.IAMInstanceProfile == nil { - providerSpec.IAMInstanceProfile = &aws.AWSResourceReference{ID: defaultAWSIAMInstanceProfile(h.clusterID)} + providerSpec.IAMInstanceProfile = &aws.AWSResourceReference{ID: defaultAWSIAMInstanceProfile(clusterID)} } if providerSpec.UserDataSecret == nil { providerSpec.UserDataSecret = &corev1.LocalObjectReference{Name: defaultUserDataSecret} @@ -282,7 +289,7 @@ func defaultAWS(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { Filters: []aws.Filter{ { Name: "tag:Name", - Values: []string{defaultAWSSecurityGroup(h.clusterID)}, + Values: []string{defaultAWSSecurityGroup(clusterID)}, }, }, }, @@ -293,7 +300,7 @@ func defaultAWS(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { providerSpec.Subnet.Filters = []aws.Filter{ { Name: "tag:Name", - Values: []string{defaultAWSSubnet(h.clusterID, providerSpec.Placement.AvailabilityZone)}, + Values: []string{defaultAWSSubnet(clusterID, providerSpec.Placement.AvailabilityZone)}, }, } } @@ -312,13 +319,17 @@ func defaultAWS(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { } func unmarshalInto(m *Machine, providerSpec interface{}) error { + if m.Spec.ProviderSpec.Value == nil { + return field.Required(field.NewPath("providerSpec", "value"), "a value must be provided") + } + if err := yaml.Unmarshal(m.Spec.ProviderSpec.Value.Raw, &providerSpec); err != nil { return field.Invalid(field.NewPath("providerSpec", "value"), providerSpec, err.Error()) } return nil } -func validateAWS(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) { +func validateAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Validating AWS providerSpec") var errs []error @@ -407,7 +418,7 @@ func validateAWS(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) { return true, nil } -func defaultAzure(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { +func defaultAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting Azure providerSpec") var errs []error @@ -423,26 +434,26 @@ func defaultAzure(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) // Vnet and Subnet need to be provided together by the user if providerSpec.Vnet == "" && providerSpec.Subnet == "" { - providerSpec.Vnet = defaultAzureVnet(h.clusterID) - providerSpec.Subnet = defaultAzureSubnet(h.clusterID) + providerSpec.Vnet = defaultAzureVnet(clusterID) + providerSpec.Subnet = defaultAzureSubnet(clusterID) // NetworkResourceGroup can be set by the user without Vnet and Subnet, // only override if they didn't set it if providerSpec.NetworkResourceGroup == "" { - providerSpec.NetworkResourceGroup = defaultAzureNetworkResourceGroup(h.clusterID) + providerSpec.NetworkResourceGroup = defaultAzureNetworkResourceGroup(clusterID) } } if providerSpec.Image.ResourceID == "" { - providerSpec.Image.ResourceID = defaultAzureImageResourceID(h.clusterID) + providerSpec.Image.ResourceID = defaultAzureImageResourceID(clusterID) } if providerSpec.ManagedIdentity == "" { - providerSpec.ManagedIdentity = defaultAzureManagedIdentiy(h.clusterID) + providerSpec.ManagedIdentity = defaultAzureManagedIdentiy(clusterID) } if providerSpec.ResourceGroup == "" { - providerSpec.ResourceGroup = defaultAzureResourceGroup(h.clusterID) + providerSpec.ResourceGroup = defaultAzureResourceGroup(clusterID) } if providerSpec.UserDataSecret == nil { @@ -483,7 +494,7 @@ func defaultAzure(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) return true, nil } -func validateAzure(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) { +func validateAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Validating Azure providerSpec") var errs []error @@ -566,7 +577,7 @@ type gcpDefaulter struct { projectID string } -func (g gcpDefaulter) defaultGCP(h *defaulterHandler, m *Machine) (bool, utilerrors.Aggregate) { +func (g gcpDefaulter) defaultGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting GCP providerSpec") var errs []error @@ -582,15 +593,15 @@ func (g gcpDefaulter) defaultGCP(h *defaulterHandler, m *Machine) (bool, utilerr if len(providerSpec.NetworkInterfaces) == 0 { providerSpec.NetworkInterfaces = append(providerSpec.NetworkInterfaces, &gcp.GCPNetworkInterface{ - Network: defaultGCPNetwork(h.clusterID), - Subnetwork: defaultGCPSubnetwork(h.clusterID), + Network: defaultGCPNetwork(clusterID), + Subnetwork: defaultGCPSubnetwork(clusterID), }) } - providerSpec.Disks = defaultGCPDisks(providerSpec.Disks, h.clusterID) + providerSpec.Disks = defaultGCPDisks(providerSpec.Disks, clusterID) if len(providerSpec.Tags) == 0 { - providerSpec.Tags = defaultGCPTags(h.clusterID) + providerSpec.Tags = defaultGCPTags(clusterID) } if providerSpec.UserDataSecret == nil { @@ -602,7 +613,7 @@ func (g gcpDefaulter) defaultGCP(h *defaulterHandler, m *Machine) (bool, utilerr } if len(providerSpec.ServiceAccounts) == 0 { - providerSpec.ServiceAccounts = defaultGCPServiceAccounts(h.clusterID, g.projectID) + providerSpec.ServiceAccounts = defaultGCPServiceAccounts(clusterID, g.projectID) } rawBytes, err := json.Marshal(providerSpec) @@ -644,7 +655,7 @@ func defaultGCPDisks(disks []*gcp.GCPDisk, clusterID string) []*gcp.GCPDisk { return disks } -func validateGCP(h *validatorHandler, m *Machine) (bool, utilerrors.Aggregate) { +func validateGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Validating GCP providerSpec") var errs []error diff --git a/pkg/apis/machine/v1beta1/machine_webhook_test.go b/pkg/apis/machine/v1beta1/machine_webhook_test.go index f476279039..1c0c43b37a 100644 --- a/pkg/apis/machine/v1beta1/machine_webhook_test.go +++ b/pkg/apis/machine/v1beta1/machine_webhook_test.go @@ -2,19 +2,614 @@ package v1beta1 import ( "encoding/json" + "fmt" "testing" + . "github.com/onsi/gomega" osconfigv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" "k8s.io/utils/pointer" aws "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" azure "sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1beta1" gcp "sigs.k8s.io/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" yaml "sigs.k8s.io/yaml" ) +func TestMachineCreation(t *testing.T) { + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-creation-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + platformType osconfigv1.PlatformType + clusterID string + expectedError string + providerSpecValue *runtime.RawExtension + }{ + { + name: "with AWS and a nil provider spec value", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with AWS and no fields set", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &aws.AWSMachineProviderConfig{}, + }, + expectedError: "providerSpec.ami: Required value: expected either providerSpec.ami.arn or providerSpec.ami.filters or providerSpec.ami.id to be populated", + }, + { + name: "with AWS and an AMI ID set", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &aws.AWSMachineProviderConfig{ + AMI: aws.AWSResourceReference{ + ID: pointer.StringPtr("ami"), + }, + }, + }, + expectedError: "", + }, + { + name: "with Azure and a nil provider spec value", + platformType: osconfigv1.AzurePlatformType, + clusterID: "azure-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with Azure and no fields set", + platformType: osconfigv1.AzurePlatformType, + clusterID: "azure-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &azure.AzureMachineProviderSpec{}, + }, + expectedError: "[providerSpec.location: Required value: location should be set to one of the supported Azure regions, providerSpec.osDisk.diskSizeGB: Invalid value: 0: diskSizeGB must be greater than zero]", + }, + { + name: "with Azure and a location and disk size set", + platformType: osconfigv1.AzurePlatformType, + clusterID: "azure-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &azure.AzureMachineProviderSpec{ + Location: "location", + OSDisk: azure.OSDisk{ + DiskSizeGB: 128, + }, + }, + }, + expectedError: "", + }, + { + name: "with GCP and a nil provider spec value", + platformType: osconfigv1.GCPPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with GCP and no fields set", + platformType: osconfigv1.GCPPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &gcp.GCPMachineProviderSpec{}, + }, + expectedError: "providerSpec.region: Required value: region is required", + }, + { + name: "with GCP and the region and zone set", + platformType: osconfigv1.GCPPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &gcp.GCPMachineProviderSpec{ + Region: "region", + Zone: "region-zone", + }, + }, + expectedError: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + platformStatus := &osconfigv1.PlatformStatus{ + Type: tc.platformType, + GCP: &osconfigv1.GCPPlatformStatus{ + ProjectID: "gcp-project-id", + }, + } + + machineDefaulter := createMachineDefaulter(platformStatus, tc.clusterID) + machineValidator := createMachineValidator(platformStatus.Type, tc.clusterID) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineValidator}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + gs.Eventually(func() (bool, error) { + resp, err := insecureHTTPClient.Get(fmt.Sprintf("https://127.0.0.1:%d", testEnv.WebhookInstallOptions.LocalServingPort)) + if err != nil { + return false, err + } + return resp.StatusCode == 404, nil + }).Should(BeTrue()) + + m := &Machine{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machine-creation-", + Namespace: namespace.Name, + }, + Spec: MachineSpec{ + ProviderSpec: ProviderSpec{ + Value: tc.providerSpecValue, + }, + }, + } + err = c.Create(ctx, m) + if err == nil { + defer func() { + gs.Expect(c.Delete(ctx, m)).To(Succeed()) + }() + } + + if tc.expectedError != "" { + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + +func TestMachineUpdate(t *testing.T) { + awsClusterID := "aws-cluster" + defaultAWSProviderSpec := &aws.AWSMachineProviderConfig{ + AMI: aws.AWSResourceReference{ + ID: pointer.StringPtr("ami"), + }, + InstanceType: defaultAWSInstanceType, + IAMInstanceProfile: &aws.AWSResourceReference{ + ID: defaultAWSIAMInstanceProfile(awsClusterID), + }, + UserDataSecret: &corev1.LocalObjectReference{Name: defaultUserDataSecret}, + CredentialsSecret: &corev1.LocalObjectReference{Name: defaultAWSCredentialsSecret}, + SecurityGroups: []aws.AWSResourceReference{ + { + Filters: []aws.Filter{ + { + Name: "tag:Name", + Values: []string{defaultAWSSecurityGroup(awsClusterID)}, + }, + }, + }, + }, + Placement: aws.Placement{ + Region: "region", + AvailabilityZone: "zone", + }, + Subnet: aws.AWSResourceReference{ + Filters: []aws.Filter{ + { + Name: "tag:Name", + Values: []string{defaultAWSSubnet(awsClusterID, "zone")}, + }, + }, + }, + } + + azureClusterID := "azure-cluster" + defaultAzureProviderSpec := &azure.AzureMachineProviderSpec{ + Location: "location", + VMSize: defaultAzureVMSize, + Vnet: defaultAzureVnet(azureClusterID), + Subnet: defaultAzureSubnet(azureClusterID), + NetworkResourceGroup: defaultAzureNetworkResourceGroup(azureClusterID), + Image: azure.Image{ + ResourceID: defaultAzureImageResourceID(azureClusterID), + }, + ManagedIdentity: defaultAzureManagedIdentiy(azureClusterID), + ResourceGroup: defaultAzureResourceGroup(azureClusterID), + UserDataSecret: &corev1.SecretReference{ + Name: defaultUserDataSecret, + Namespace: defaultSecretNamespace, + }, + CredentialsSecret: &corev1.SecretReference{ + Name: defaultAzureCredentialsSecret, + Namespace: defaultSecretNamespace, + }, + OSDisk: azure.OSDisk{ + DiskSizeGB: 128, + OSType: defaultAzureOSDiskOSType, + ManagedDisk: azure.ManagedDisk{ + StorageAccountType: defaultAzureOSDiskStorageType, + }, + }, + } + + gcpClusterID := "gcp-cluster" + gcpProjectID := "gcp-project-id" + defaultGCPProviderSpec := &gcp.GCPMachineProviderSpec{ + Region: "region", + Zone: "region-zone", + MachineType: defaultGCPMachineType, + NetworkInterfaces: []*gcp.GCPNetworkInterface{ + { + Network: defaultGCPNetwork(gcpClusterID), + Subnetwork: defaultGCPSubnetwork(gcpClusterID), + }, + }, + Disks: []*gcp.GCPDisk{ + { + AutoDelete: true, + Boot: true, + SizeGb: defaultGCPDiskSizeGb, + Type: defaultGCPDiskType, + Image: defaultGCPDiskImage(gcpClusterID), + }, + }, + ServiceAccounts: defaultGCPServiceAccounts(gcpClusterID, gcpProjectID), + Tags: defaultGCPTags(gcpClusterID), + UserDataSecret: &corev1.LocalObjectReference{ + Name: defaultUserDataSecret, + }, + CredentialsSecret: &corev1.LocalObjectReference{ + Name: defaultGCPCredentialsSecret, + }, + } + + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-update-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + platformType osconfigv1.PlatformType + clusterID string + expectedError string + baseProviderSpecValue *runtime.RawExtension + updatedProviderSpecValue func() *runtime.RawExtension + }{ + { + name: "with a valid AWS ProviderSpec", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with an AWS ProviderSpec, removing the instance type", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.InstanceType = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.instanceType: Required value: expected providerSpec.instanceType to be populated", + }, + { + name: "with an AWS ProviderSpec, removing the instance profile", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.IAMInstanceProfile = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.iamInstanceProfile: Required value: expected providerSpec.iamInstanceProfile to be populated", + }, + { + name: "with an AWS ProviderSpec, removing the user data secret", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.UserDataSecret = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.userDataSecret: Required value: expected providerSpec.userDataSecret to be populated", + }, + { + name: "with a valid Azure ProviderSpec", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with an Azure ProviderSpec, removing the vm size", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.VMSize = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.vmSize: Required value: vmSize should be set to one of the supported Azure VM sizes", + }, + { + name: "with an Azure ProviderSpec, removing the subnet", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.Subnet = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.subnet: Required value: must provide a subnet when a virtual network is specified", + }, + { + name: "with an Azure ProviderSpec, removing the credentials secret", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.CredentialsSecret = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.credentialsSecret: Required value: credentialsSecret must be provided", + }, + { + name: "with a valid GCP ProviderSpec", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with a GCP ProviderSpec, removing the region", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Region = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.region: Required value: region is required", + }, + { + name: "with a GCP ProviderSpec, and an invalid region", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Zone = "zone" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.zone: Invalid value: \"zone\": zone not in configured region (region)", + }, + { + name: "with a GCP ProviderSpec, removing the disks", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Disks = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.disks: Required value: at least 1 disk is required", + }, + { + name: "with a GCP ProviderSpec, removing the service accounts", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.ServiceAccounts = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.serviceAccounts: Invalid value: \"0 service accounts supplied\": exactly 1 service account must be supplied", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + platformStatus := &osconfigv1.PlatformStatus{ + Type: tc.platformType, + GCP: &osconfigv1.GCPPlatformStatus{ + ProjectID: gcpProjectID, + }, + } + + machineDefaulter := createMachineDefaulter(platformStatus, tc.clusterID) + machineValidator := createMachineValidator(platformStatus.Type, tc.clusterID) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machine", &webhook.Admission{Handler: machineValidator}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + gs.Eventually(func() (bool, error) { + resp, err := insecureHTTPClient.Get(fmt.Sprintf("https://127.0.0.1:%d", testEnv.WebhookInstallOptions.LocalServingPort)) + if err != nil { + return false, err + } + return resp.StatusCode == 404, nil + }).Should(BeTrue()) + + m := &Machine{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machine-creation-", + Namespace: namespace.Name, + }, + Spec: MachineSpec{ + ProviderSpec: ProviderSpec{ + Value: tc.baseProviderSpecValue, + }, + }, + } + err = c.Create(ctx, m) + gs.Expect(err).ToNot(HaveOccurred()) + defer func() { + gs.Expect(c.Delete(ctx, m)).To(Succeed()) + }() + + m.Spec.ProviderSpec.Value = tc.updatedProviderSpecValue() + err = c.Update(ctx, m) + if tc.expectedError != "" { + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + func TestValidateAWSProviderSpec(t *testing.T) { testCases := []struct { @@ -124,7 +719,7 @@ func TestValidateAWSProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } @@ -215,7 +810,7 @@ func TestDefaultAWSProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } @@ -446,7 +1041,7 @@ func TestValidateAzureProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } @@ -563,7 +1158,7 @@ func TestDefaultAzureProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } @@ -832,7 +1427,7 @@ func TestValidateGCPProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } @@ -941,7 +1536,7 @@ func TestDefaultGCPProviderSpec(t *testing.T) { } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - ok, err := h.webhookOperations(h, m) + ok, err := h.webhookOperations(m, h.clusterID) if ok != tc.expectedOk { t.Errorf("expected: %v, got: %v", tc.expectedOk, ok) } diff --git a/pkg/apis/machine/v1beta1/machineset_cp_webhook.go b/pkg/apis/machine/v1beta1/machineset_cp_webhook.go new file mode 100644 index 0000000000..6990bc108e --- /dev/null +++ b/pkg/apis/machine/v1beta1/machineset_cp_webhook.go @@ -0,0 +1,84 @@ +package v1beta1 + +import ( + "context" + "fmt" + "net/http" + + admissionv1beta1 "k8s.io/api/admission/v1beta1" + "k8s.io/klog" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// MachineSetCPHandler validates ControlPlane MachineSet API resources. +// implements type Handler interface. +// https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler +type MachineSetCPHandler struct { + decoder *admission.Decoder +} + +// NewMachineSetCPValidator returns a new MachineSetCPHandler. +func NewMachineSetCPValidator() (*MachineSetCPHandler, error) { + return createMachineSetCPValidator(), nil +} + +func createMachineSetCPValidator() *MachineSetCPHandler { + return &MachineSetCPHandler{} +} + +// InjectDecoder injects the decoder. +func (v *MachineSetCPHandler) InjectDecoder(d *admission.Decoder) error { + v.decoder = d + return nil +} + +// Handle handles HTTP requests for admission webhook servers. +func (v *MachineSetCPHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + oldMS := &MachineSet{} + + // Delete requests, the req.Object is empty. + if err := v.decoder.DecodeRaw(req.OldObject, oldMS); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + klog.V(3).Infof("Validate webhook called for CP MachineSets: %s", oldMS.GetName()) + + newMS := &MachineSet{} + if req.Operation != admissionv1beta1.Delete { + if err := v.decoder.Decode(req, newMS); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + } + + // Succeed if deleting non-CP MachineSet or Updating a non-CP MachineSet + // and the user is not attempting to change it to a CP MachineSet. + if !isCPMS(oldMS) && (req.Operation == admissionv1beta1.Delete || !isCPMS(newMS)) { + return admission.Allowed("MachineSet is Not Control Plane.") + } + + // If the user is updating a CP MachineSet, as long as machine role is + // unchanged, we're ok. + if req.Operation != admissionv1beta1.Delete && isCPMS(newMS) && isCPMS(oldMS) { + return admission.Allowed("Control Plane MachineSet is Valid.") + } + + // User is peforming an unallowed operation + + // TODO(michaelgugino): Ensure we account for MachineDeployment ownership + // of a CP machineset in the future if we use them. + return admission.Denied(fmt.Sprintf("Requested %v of Control Plane MachineSet Not Allowed.", req.Operation)) + +} + +func isCPMS(ms *MachineSet) bool { + if ms.Spec.Template.ObjectMeta.Labels == nil { + return false + } + val, ok := ms.Spec.Template.ObjectMeta.Labels["machine.openshift.io/cluster-api-machine-role"] + if ok { + if val == "master" { + return true + } + } + return false +} diff --git a/pkg/apis/machine/v1beta1/machineset_types_test.go b/pkg/apis/machine/v1beta1/machineset_types_test.go index 53d6f3eca5..e5495625f0 100644 --- a/pkg/apis/machine/v1beta1/machineset_types_test.go +++ b/pkg/apis/machine/v1beta1/machineset_types_test.go @@ -32,12 +32,40 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" + + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" ) func TestStorageMachineSet(t *testing.T) { key := types.NamespacedName{Name: "foo", Namespace: "default"} created := &MachineSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}} + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + GracefulShutdownTimeout: &managerGracePeriodDuration, + }) + + gs.Expect(err).ToNot(HaveOccurred()) + + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + // Test Create fetched := &MachineSet{} if err := c.Create(context.TODO(), created); err != nil { diff --git a/pkg/apis/machine/v1beta1/machineset_webhook.go b/pkg/apis/machine/v1beta1/machineset_webhook.go new file mode 100644 index 0000000000..595f406028 --- /dev/null +++ b/pkg/apis/machine/v1beta1/machineset_webhook.go @@ -0,0 +1,135 @@ +package v1beta1 + +import ( + "context" + "encoding/json" + "net/http" + + osconfigv1 "github.com/openshift/api/config/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/klog" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// machineSetValidatorHandler validates MachineSet API resources. +// implements type Handler interface. +// https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler +type machineSetValidatorHandler struct { + *admissionHandler +} + +// machineSetDefaulterHandler defaults MachineSet API resources. +// implements type Handler interface. +// https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook/admission#Handler +type machineSetDefaulterHandler struct { + *admissionHandler +} + +// NewMachineSetValidator returns a new machineSetValidatorHandler. +func NewMachineSetValidator() (*machineSetValidatorHandler, error) { + infra, err := getInfra() + if err != nil { + return nil, err + } + + return createMachineSetValidator(infra.Status.PlatformStatus.Type, infra.Status.InfrastructureName), nil +} + +func createMachineSetValidator(platform osconfigv1.PlatformType, clusterID string) *machineSetValidatorHandler { + return &machineSetValidatorHandler{ + admissionHandler: &admissionHandler{ + clusterID: clusterID, + webhookOperations: getMachineValidatorOperation(platform), + }, + } +} + +// NewMachineSetDefaulter returns a new machineSetDefaulterHandler. +func NewMachineSetDefaulter() (*machineSetDefaulterHandler, error) { + infra, err := getInfra() + if err != nil { + return nil, err + } + + return createMachineSetDefaulter(infra.Status.PlatformStatus, infra.Status.InfrastructureName), nil +} + +func createMachineSetDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID string) *machineSetDefaulterHandler { + return &machineSetDefaulterHandler{ + admissionHandler: &admissionHandler{ + clusterID: clusterID, + webhookOperations: getMachineDefaulterOperation(platformStatus), + }, + } +} + +// Handle handles HTTP requests for admission webhook servers. +func (h *machineSetValidatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + ms := &MachineSet{} + + if err := h.decoder.Decode(req, ms); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + klog.V(3).Infof("Validate webhook called for MachineSet: %s", ms.GetName()) + + if ok, err := h.validateMachineSet(ms); !ok { + return admission.Denied(err.Error()) + } + + return admission.Allowed("MachineSet valid") +} + +// Handle handles HTTP requests for admission webhook servers. +func (h *machineSetDefaulterHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + ms := &MachineSet{} + + if err := h.decoder.Decode(req, ms); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + klog.V(3).Infof("Mutate webhook called for MachineSet: %s", ms.GetName()) + + if ok, err := h.defaultMachineSet(ms); !ok { + return admission.Denied(err.Error()) + } + + marshaledMachineSet, err := json.Marshal(ms) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + return admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachineSet) +} + +func (h *machineSetValidatorHandler) validateMachineSet(ms *MachineSet) (bool, utilerrors.Aggregate) { + var errs []error + + // Create a Machine from the MachineSet and validate the Machine template + m := &Machine{Spec: ms.Spec.Template.Spec} + if ok, err := h.webhookOperations(m, h.clusterID); !ok { + errs = append(errs, err.Errors()...) + } + + if len(errs) > 0 { + return false, utilerrors.NewAggregate(errs) + } + return true, nil +} + +func (h *machineSetDefaulterHandler) defaultMachineSet(ms *MachineSet) (bool, utilerrors.Aggregate) { + var errs []error + + // Create a Machine from the MachineSet and default the Machine template + m := &Machine{Spec: ms.Spec.Template.Spec} + if ok, err := h.webhookOperations(m, h.clusterID); !ok { + errs = append(errs, err.Errors()...) + } else { + // Restore the defaulted template + ms.Spec.Template.Spec = m.Spec + } + + if len(errs) > 0 { + return false, utilerrors.NewAggregate(errs) + } + return true, nil +} diff --git a/pkg/apis/machine/v1beta1/machineset_webhook_test.go b/pkg/apis/machine/v1beta1/machineset_webhook_test.go new file mode 100644 index 0000000000..0d7c1755b7 --- /dev/null +++ b/pkg/apis/machine/v1beta1/machineset_webhook_test.go @@ -0,0 +1,941 @@ +package v1beta1 + +import ( + "context" + "fmt" + "testing" + + . "github.com/onsi/gomega" + osconfigv1 "github.com/openshift/api/config/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "k8s.io/utils/pointer" + aws "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" + azure "sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1beta1" + gcp "sigs.k8s.io/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +func TestMachineSetCreation(t *testing.T) { + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machineset-creation-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + platformType osconfigv1.PlatformType + clusterID string + expectedError string + providerSpecValue *runtime.RawExtension + }{ + { + name: "with AWS and a nil provider spec value", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with AWS and no fields set", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &aws.AWSMachineProviderConfig{}, + }, + expectedError: "providerSpec.ami: Required value: expected either providerSpec.ami.arn or providerSpec.ami.filters or providerSpec.ami.id to be populated", + }, + { + name: "with AWS and an AMI ID set", + platformType: osconfigv1.AWSPlatformType, + clusterID: "aws-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &aws.AWSMachineProviderConfig{ + AMI: aws.AWSResourceReference{ + ID: pointer.StringPtr("ami"), + }, + }, + }, + expectedError: "", + }, + { + name: "with Azure and a nil provider spec value", + platformType: osconfigv1.AWSPlatformType, + clusterID: "azure-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with Azure and no fields set", + platformType: osconfigv1.AzurePlatformType, + clusterID: "azure-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &azure.AzureMachineProviderSpec{}, + }, + expectedError: "[providerSpec.location: Required value: location should be set to one of the supported Azure regions, providerSpec.osDisk.diskSizeGB: Invalid value: 0: diskSizeGB must be greater than zero]", + }, + { + name: "with Azure and a location and disk size set", + platformType: osconfigv1.AzurePlatformType, + clusterID: "azure-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &azure.AzureMachineProviderSpec{ + Location: "location", + OSDisk: azure.OSDisk{ + DiskSizeGB: 128, + }, + }, + }, + expectedError: "", + }, + { + name: "with GCP and a nil provider spec value", + platformType: osconfigv1.AWSPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: nil, + expectedError: "providerSpec.value: Required value: a value must be provided", + }, + { + name: "with GCP and no fields set", + platformType: osconfigv1.GCPPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &gcp.GCPMachineProviderSpec{}, + }, + expectedError: "providerSpec.region: Required value: region is required", + }, + { + name: "with GCP and the region and zone set", + platformType: osconfigv1.GCPPlatformType, + clusterID: "gcp-cluster", + providerSpecValue: &runtime.RawExtension{ + Object: &gcp.GCPMachineProviderSpec{ + Region: "region", + Zone: "region-zone", + }, + }, + expectedError: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + GracefulShutdownTimeout: &managerGracePeriodDuration, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + platformStatus := &osconfigv1.PlatformStatus{ + Type: tc.platformType, + GCP: &osconfigv1.GCPPlatformStatus{ + ProjectID: "gcp-project-id", + }, + } + + machineSetDefaulter := createMachineSetDefaulter(platformStatus, tc.clusterID) + machineSetValidator := createMachineSetValidator(platformStatus.Type, tc.clusterID) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetValidator}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + gs.Eventually(func() (bool, error) { + resp, err := insecureHTTPClient.Get(fmt.Sprintf("https://127.0.0.1:%d", testEnv.WebhookInstallOptions.LocalServingPort)) + if err != nil { + return false, err + } + return resp.StatusCode == 404, nil + }).Should(BeTrue()) + + ms := &MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machineset-creation-", + Namespace: namespace.Name, + }, + Spec: MachineSetSpec{ + Template: MachineTemplateSpec{ + Spec: MachineSpec{ + ProviderSpec: ProviderSpec{ + Value: tc.providerSpecValue, + }, + }, + }, + }, + } + err = c.Create(ctx, ms) + if err == nil { + defer func() { + gs.Expect(c.Delete(ctx, ms)).To(Succeed()) + }() + } + + if tc.expectedError != "" { + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + +func TestMachineSetUpdate(t *testing.T) { + awsClusterID := "aws-cluster" + defaultAWSProviderSpec := &aws.AWSMachineProviderConfig{ + AMI: aws.AWSResourceReference{ + ID: pointer.StringPtr("ami"), + }, + InstanceType: defaultAWSInstanceType, + IAMInstanceProfile: &aws.AWSResourceReference{ + ID: defaultAWSIAMInstanceProfile(awsClusterID), + }, + UserDataSecret: &corev1.LocalObjectReference{Name: defaultUserDataSecret}, + CredentialsSecret: &corev1.LocalObjectReference{Name: defaultAWSCredentialsSecret}, + SecurityGroups: []aws.AWSResourceReference{ + { + Filters: []aws.Filter{ + { + Name: "tag:Name", + Values: []string{defaultAWSSecurityGroup(awsClusterID)}, + }, + }, + }, + }, + Placement: aws.Placement{ + Region: "region", + AvailabilityZone: "zone", + }, + Subnet: aws.AWSResourceReference{ + Filters: []aws.Filter{ + { + Name: "tag:Name", + Values: []string{defaultAWSSubnet(awsClusterID, "zone")}, + }, + }, + }, + } + + azureClusterID := "azure-cluster" + defaultAzureProviderSpec := &azure.AzureMachineProviderSpec{ + Location: "location", + VMSize: defaultAzureVMSize, + Vnet: defaultAzureVnet(azureClusterID), + Subnet: defaultAzureSubnet(azureClusterID), + NetworkResourceGroup: defaultAzureNetworkResourceGroup(azureClusterID), + Image: azure.Image{ + ResourceID: defaultAzureImageResourceID(azureClusterID), + }, + ManagedIdentity: defaultAzureManagedIdentiy(azureClusterID), + ResourceGroup: defaultAzureResourceGroup(azureClusterID), + UserDataSecret: &corev1.SecretReference{ + Name: defaultUserDataSecret, + Namespace: defaultSecretNamespace, + }, + CredentialsSecret: &corev1.SecretReference{ + Name: defaultAzureCredentialsSecret, + Namespace: defaultSecretNamespace, + }, + OSDisk: azure.OSDisk{ + DiskSizeGB: 128, + OSType: defaultAzureOSDiskOSType, + ManagedDisk: azure.ManagedDisk{ + StorageAccountType: defaultAzureOSDiskStorageType, + }, + }, + } + + gcpClusterID := "gcp-cluster" + gcpProjectID := "gcp-project-id" + defaultGCPProviderSpec := &gcp.GCPMachineProviderSpec{ + Region: "region", + Zone: "region-zone", + MachineType: defaultGCPMachineType, + NetworkInterfaces: []*gcp.GCPNetworkInterface{ + { + Network: defaultGCPNetwork(gcpClusterID), + Subnetwork: defaultGCPSubnetwork(gcpClusterID), + }, + }, + Disks: []*gcp.GCPDisk{ + { + AutoDelete: true, + Boot: true, + SizeGb: defaultGCPDiskSizeGb, + Type: defaultGCPDiskType, + Image: defaultGCPDiskImage(gcpClusterID), + }, + }, + ServiceAccounts: defaultGCPServiceAccounts(gcpClusterID, gcpProjectID), + Tags: defaultGCPTags(gcpClusterID), + UserDataSecret: &corev1.LocalObjectReference{ + Name: defaultUserDataSecret, + }, + CredentialsSecret: &corev1.LocalObjectReference{ + Name: defaultGCPCredentialsSecret, + }, + } + + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machineset-update-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + platformType osconfigv1.PlatformType + clusterID string + expectedError string + baseProviderSpecValue *runtime.RawExtension + updatedProviderSpecValue func() *runtime.RawExtension + }{ + { + name: "with a valid AWS ProviderSpec", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with an AWS ProviderSpec, removing the instance type", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.InstanceType = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.instanceType: Required value: expected providerSpec.instanceType to be populated", + }, + { + name: "with an AWS ProviderSpec, removing the instance profile", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.IAMInstanceProfile = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.iamInstanceProfile: Required value: expected providerSpec.iamInstanceProfile to be populated", + }, + { + name: "with an AWS ProviderSpec, removing the user data secret", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAWSProviderSpec.DeepCopy() + object.UserDataSecret = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.userDataSecret: Required value: expected providerSpec.userDataSecret to be populated", + }, + { + name: "with a valid Azure ProviderSpec", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with an Azure ProviderSpec, removing the vm size", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.VMSize = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.vmSize: Required value: vmSize should be set to one of the supported Azure VM sizes", + }, + { + name: "with an Azure ProviderSpec, removing the subnet", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.Subnet = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.subnet: Required value: must provide a subnet when a virtual network is specified", + }, + { + name: "with an Azure ProviderSpec, removing the credentials secret", + platformType: osconfigv1.AzurePlatformType, + clusterID: azureClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultAzureProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultAzureProviderSpec.DeepCopy() + object.CredentialsSecret = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.credentialsSecret: Required value: credentialsSecret must be provided", + }, + { + name: "with a valid GCP ProviderSpec", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + return &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + } + }, + expectedError: "", + }, + { + name: "with a GCP ProviderSpec, removing the region", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Region = "" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.region: Required value: region is required", + }, + { + name: "with a GCP ProviderSpec, and an invalid region", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Zone = "zone" + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.zone: Invalid value: \"zone\": zone not in configured region (region)", + }, + { + name: "with a GCP ProviderSpec, removing the disks", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.Disks = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.disks: Required value: at least 1 disk is required", + }, + { + name: "with a GCP ProviderSpec, removing the service accounts", + platformType: osconfigv1.GCPPlatformType, + clusterID: gcpClusterID, + baseProviderSpecValue: &runtime.RawExtension{ + Object: defaultGCPProviderSpec.DeepCopy(), + }, + updatedProviderSpecValue: func() *runtime.RawExtension { + object := defaultGCPProviderSpec.DeepCopy() + object.ServiceAccounts = nil + return &runtime.RawExtension{ + Object: object, + } + }, + expectedError: "providerSpec.serviceAccounts: Invalid value: \"0 service accounts supplied\": exactly 1 service account must be supplied", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + GracefulShutdownTimeout: &managerGracePeriodDuration, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + platformStatus := &osconfigv1.PlatformStatus{ + Type: tc.platformType, + GCP: &osconfigv1.GCPPlatformStatus{ + ProjectID: gcpProjectID, + }, + } + + machineSetDefaulter := createMachineSetDefaulter(platformStatus, tc.clusterID) + machineSetValidator := createMachineSetValidator(platformStatus.Type, tc.clusterID) + mgr.GetWebhookServer().Register("/mutate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetDefaulter}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset", &webhook.Admission{Handler: machineSetValidator}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + gs.Eventually(func() (bool, error) { + resp, err := insecureHTTPClient.Get(fmt.Sprintf("https://127.0.0.1:%d", testEnv.WebhookInstallOptions.LocalServingPort)) + if err != nil { + return false, err + } + return resp.StatusCode == 404, nil + }).Should(BeTrue()) + + ms := &MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machineset-update-", + Namespace: namespace.Name, + }, + Spec: MachineSetSpec{ + Template: MachineTemplateSpec{ + Spec: MachineSpec{ + ProviderSpec: ProviderSpec{ + Value: tc.baseProviderSpecValue, + }, + }, + }, + }, + } + err = c.Create(ctx, ms) + gs.Expect(err).ToNot(HaveOccurred()) + defer func() { + gs.Expect(c.Delete(ctx, ms)).To(Succeed()) + }() + + ms.Spec.Template.Spec.ProviderSpec.Value = tc.updatedProviderSpecValue() + err = c.Update(ctx, ms) + if tc.expectedError != "" { + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + +func TestCPMachineSetDelete(t *testing.T) { + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machineset-cp-delete-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + expectedError string + objectMeta ObjectMeta + }{ + { + name: "is not CP MachineSet", + expectedError: "", + objectMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + }, + { + name: "is not CP MachineSet, no labels", + expectedError: "", + objectMeta: ObjectMeta{}, + }, + { + name: "is CP MachineSet", + expectedError: "Requested DELETE of Control Plane MachineSet Not Allowed.", + objectMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + GracefulShutdownTimeout: &managerGracePeriodDuration, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + machineSetCPDeletionValidator, err := NewMachineSetCPValidator() + gs.Expect(err).ToNot(HaveOccurred()) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: machineSetCPDeletionValidator}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + ms := &MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machineset-cp-deletion-", + Namespace: namespace.Name, + }, + Spec: MachineSetSpec{ + Template: MachineTemplateSpec{ + Spec: MachineSpec{}, + }, + }, + } + + ms.Spec.Template.ObjectMeta = tc.objectMeta + gs.Expect(c.Create(ctx, ms)).To(Succeed()) + + err = c.Delete(ctx, ms) + + if tc.expectedError != "" { + defer func() { + ms.Spec.Template.ObjectMeta.Labels["machine.openshift.io/cluster-api-machine-role"] = "worker" + gs.Expect(c.Update(ctx, ms)).To(Succeed()) + gs.Expect(c.Delete(ctx, ms)).To(Succeed()) + }() + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + +func TestCPMachineSetUpdate(t *testing.T) { + g := NewWithT(t) + + // Override config getter + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + defer func() { + ctrl.GetConfig = config.GetConfig + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machineset-cp-update-test", + }, + } + g.Expect(c.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(c.Delete(ctx, namespace)).To(Succeed()) + }() + + testCases := []struct { + name string + expectedError string + originalMeta ObjectMeta + updateMeta ObjectMeta + }{ + { + name: "is not CP MachineSet, not becoming CP", + expectedError: "", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + updateMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker2", + }, + }, + }, + { + name: "is not CP MachineSet, labels removed", + expectedError: "", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + updateMeta: ObjectMeta{ + Labels: nil, + }, + }, + { + name: "no Lables (non-CP) to non-CP labels", + expectedError: "", + originalMeta: ObjectMeta{ + Labels: nil, + }, + updateMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + }, + { + name: "CP MachineSet, add another label", + expectedError: "", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + }, + }, + updateMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + "secondlabel": "second label value", + }, + }, + }, + { + name: "CP MachineSet, try to change role", + expectedError: "Requested UPDATE of Control Plane MachineSet Not Allowed.", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + }, + }, + updateMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + }, + { + name: "CP MachineSet, try to remove labels", + expectedError: "Requested UPDATE of Control Plane MachineSet Not Allowed.", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + }, + }, + updateMeta: ObjectMeta{ + Labels: nil, + }, + }, + { + name: "Non CP become CP", + expectedError: "Requested UPDATE of Control Plane MachineSet Not Allowed.", + originalMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + }, + updateMeta: ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "master", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + mgr, err := manager.New(cfg, manager.Options{ + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + GracefulShutdownTimeout: &managerGracePeriodDuration, + }) + gs.Expect(err).ToNot(HaveOccurred()) + + machineSetCPUpdateValidator, err := NewMachineSetCPValidator() + gs.Expect(err).ToNot(HaveOccurred()) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-delete", &webhook.Admission{Handler: createMachineSetMockHandler(true)}) + mgr.GetWebhookServer().Register("/validate-machine-openshift-io-v1beta1-machineset-cp-update", &webhook.Admission{Handler: machineSetCPUpdateValidator}) + + done := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + gs.Expect(mgr.Start(done)).To(Succeed()) + close(stopped) + }() + defer func() { + close(done) + <-stopped + }() + + ms := &MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "machineset-cp-update-", + Namespace: namespace.Name, + }, + Spec: MachineSetSpec{ + Template: MachineTemplateSpec{ + Spec: MachineSpec{}, + }, + }, + } + + ms.Spec.Template.ObjectMeta = tc.originalMeta + err = c.Create(ctx, ms) + gs.Expect(err).ToNot(HaveOccurred()) + defer func() { + gs.Expect(c.Delete(ctx, ms)).To(Succeed()) + }() + + ms.Spec.Template.ObjectMeta = tc.updateMeta + + err = c.Update(ctx, ms) + + if tc.expectedError != "" { + gs.Expect(err).ToNot(BeNil()) + gs.Expect(apierrors.ReasonForError(err)).To(BeEquivalentTo(tc.expectedError)) + } else { + gs.Expect(err).To(BeNil()) + } + }) + } +} + +type MachineSetMockHandler struct { + decoder *admission.Decoder + shouldAdmit bool +} + +func createMachineSetMockHandler(shouldAdmit bool) *MachineSetMockHandler { + return &MachineSetMockHandler{shouldAdmit: shouldAdmit} +} + +// InjectDecoder injects the decoder. +func (h *MachineSetMockHandler) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + return nil +} + +// Handle handles HTTP requests for admission webhook servers. +func (h *MachineSetMockHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + if h.shouldAdmit { + return admission.Allowed("OK") + } + return admission.Denied("Not OK") +} diff --git a/pkg/apis/machine/v1beta1/v1beta1_suite_test.go b/pkg/apis/machine/v1beta1/v1beta1_suite_test.go index fff26e5d4a..889634dbba 100644 --- a/pkg/apis/machine/v1beta1/v1beta1_suite_test.go +++ b/pkg/apis/machine/v1beta1/v1beta1_suite_test.go @@ -17,13 +17,17 @@ limitations under the License. package v1beta1 import ( + "context" + "crypto/tls" "log" + "net/http" "os" "path/filepath" "testing" "time" fuzz "github.com/google/gofuzz" + osconfigv1 "github.com/openshift/api/config/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/kubernetes/scheme" @@ -32,12 +36,30 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" ) -var cfg *rest.Config -var c client.Client +var ( + cfg *rest.Config + c client.Client + ctx = context.Background() + testEnv *envtest.Environment + insecureHTTPClient = http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + managerGracePeriodDuration = time.Duration(1 * time.Second) +) func TestMain(m *testing.M) { - t := &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "..", "install")}, + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "..", "install"), + filepath.Join("..", "..", "..", "..", "vendor", "github.com", "openshift", "api", "config", "v1"), + }, + WebhookInstallOptions: envtest.WebhookInstallOptions{ + DirectoryPaths: []string{filepath.Join("..", "..", "..", "..", "install")}, + }, } err := SchemeBuilder.AddToScheme(scheme.Scheme) @@ -45,7 +67,12 @@ func TestMain(m *testing.M) { log.Fatal(err) } - if cfg, err = t.Start(); err != nil { + err = osconfigv1.AddToScheme(scheme.Scheme) + if err != nil { + log.Fatal(err) + } + + if cfg, err = testEnv.Start(); err != nil { log.Fatal(err) } @@ -54,7 +81,7 @@ func TestMain(m *testing.M) { } code := m.Run() - t.Stop() + testEnv.Stop() os.Exit(code) }