From 30c51518a9429b6493fe549a2bfa4dd41f023300 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Mon, 26 Jun 2023 17:32:50 -0700 Subject: [PATCH 1/6] webhook handler for v1beta1 MC --- .../placement.karavel.io_memberclusters.yaml | 305 ++++++++++++++++++ .../fleetresourcehandler_webhook.go | 48 ++- pkg/webhook/webhook.go | 35 +- test/e2e/e2e_test.go | 60 ++-- test/e2e/webhook_test.go | 30 +- 5 files changed, 421 insertions(+), 57 deletions(-) create mode 100644 charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml diff --git a/charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml b/charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml new file mode 100644 index 000000000..fa1305ee1 --- /dev/null +++ b/charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml @@ -0,0 +1,305 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.11.4 + name: memberclusters.placement.karavel.io +spec: + group: placement.karavel.io + names: + categories: + - fleet + kind: MemberCluster + listKind: MemberClusterList + plural: memberclusters + shortNames: + - mc + singular: membercluster + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=="Joined")].status + name: Joined + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1beta1 + schema: + openAPIV3Schema: + description: MemberCluster is a resource created in the hub cluster to represent + a member cluster within a fleet. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: The desired state of MemberCluster. + properties: + heartbeatPeriodSeconds: + default: 60 + description: 'How often (in seconds) for the member cluster to send + a heartbeat to the hub cluster. Default: 60 seconds. Min: 1 second. + Max: 10 minutes.' + format: int32 + maximum: 600 + minimum: 1 + type: integer + identity: + description: The identity used by the member cluster to access the + hub cluster. The hub agents deployed on the hub cluster will automatically + grant the minimal required permissions to this identity for the + member agents deployed on the member cluster to access the hub cluster. + properties: + apiGroup: + description: APIGroup holds the API group of the referenced subject. + Defaults to "" for ServiceAccount subjects. Defaults to "rbac.authorization.k8s.io" + for User and Group subjects. + type: string + kind: + description: Kind of object being referenced. Values defined by + this API group are "User", "Group", and "ServiceAccount". If + the Authorizer does not recognized the kind value, the Authorizer + should report an error. + type: string + name: + description: Name of the object being referenced. + type: string + namespace: + description: Namespace of the referenced object. If the object + kind is non-namespace, such as "User" or "Group", and this value + is not empty the Authorizer should report an error. + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + state: + description: 'The desired state of the member cluster. Possible values: + Join, Leave.' + type: string + required: + - identity + - state + type: object + status: + description: The observed status of MemberCluster. + properties: + agentStatus: + description: AgentStatus is an array of current observed status, each + corresponding to one member agent running in the member cluster. + items: + description: AgentStatus defines the observed status of the member + agent of the given type. + properties: + conditions: + description: Conditions is an array of current observed conditions + for the member agent. + items: + description: "Condition contains details for one aspect of + the current state of this API Resource. --- This struct + is intended for direct use as an array at the field path + .status.conditions. For example, \n type FooStatus struct{ + // Represents the observations of a foo's current state. + // Known .status.conditions.type are: \"Available\", \"Progressing\", + and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields + }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should + be when the underlying condition changed. If that is + not known, then using the time when the API field changed + is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, + if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the + current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier + indicating the reason for the condition's last transition. + Producers of specific condition types may define expected + values and meanings for this field, and whether the + values are considered a guaranteed API. The value should + be a CamelCase string. This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across + resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability + to deconflict is important. The regex it matches is + (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + lastReceivedHeartbeat: + description: Last time we received a heartbeat from the member + agent. + format: date-time + type: string + type: + description: Type of the member agent. + type: string + required: + - type + type: object + type: array + conditions: + description: Conditions is an array of current observed conditions + for the member cluster. + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + resourceUsage: + description: The current observed resource usage of the member cluster. + It is copied from the corresponding InternalMemberCluster object. + properties: + allocatable: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Allocatable represents the total resources of all + the nodes on a member cluster that are available for scheduling. + type: object + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Capacity represents the total resource capacity of + all the nodes on a member cluster. + type: object + observationTime: + description: When the resource usage is observed. + format: date-time + type: string + type: object + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index bc9650b03..74f0017b8 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/webhook/validation" ) @@ -48,9 +49,14 @@ func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Reque case createCRDGVK(): klog.V(2).InfoS("handling CRD resource", "GVK", createCRDGVK()) response = v.handleCRD(req) - case createMemberClusterGVK(): - klog.V(2).InfoS("handling Member cluster resource", "GVK", createMemberClusterGVK()) - response = v.handleMemberCluster(ctx, req) + case createV1alpha1MemberClusterGVK(): + gvk := createV1alpha1MemberClusterGVK() + klog.V(2).InfoS("handling Member cluster resource", "GVK", gvk) + response = v.handleMemberCluster(ctx, req, gvk) + case createV1beta1MemberClusterGVK(): + gvk := createV1beta1MemberClusterGVK() + klog.V(2).InfoS("handling Member cluster resource", "GVK", gvk) + response = v.handleMemberCluster(ctx, req, gvk) default: klog.V(2).InfoS("resource is not monitored by fleet resource validator webhook", "GVK", req.Kind.String()) response = admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify resource with GVK: %s", req.UserInfo.Username, req.UserInfo.Groups, req.Kind.String())) @@ -73,17 +79,29 @@ func (v *fleetResourceValidator) handleCRD(req admission.Request) admission.Resp return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) } -func (v *fleetResourceValidator) handleMemberCluster(ctx context.Context, req admission.Request) admission.Response { - var mc fleetv1alpha1.MemberCluster - if err := v.decodeRequestObject(req, &mc); err != nil { - return admission.Errored(http.StatusBadRequest, err) +func (v *fleetResourceValidator) handleMemberCluster(ctx context.Context, req admission.Request, gvk metav1.GroupVersionKind) admission.Response { + var mcName string + switch gvk { + case createV1alpha1MemberClusterGVK(): + var mc fleetv1alpha1.MemberCluster + if err := v.decodeRequestObject(req, &mc); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + mcName = mc.Name + case createV1beta1MemberClusterGVK(): + var mc fleetv1beta1.MemberCluster + if err := v.decodeRequestObject(req, &mc); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + mcName = mc.Name + default: + return admission.Denied(fmt.Sprintf("unsupported member cluster group: %s, version: %s & kind: %s", gvk.Group, gvk.Version, gvk.Kind)) } - if !validation.ValidateUserForFleetCR(ctx, v.client, v.whiteListedUsers, req.UserInfo) { - return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify member cluster CR: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) + return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify member cluster CR: %s", req.UserInfo.Username, req.UserInfo.Groups, mcName)) } klog.V(2).InfoS("user in groups is allowed to modify member cluster CR", "user", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify member cluster: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) + return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify member cluster: %s", req.UserInfo.Username, req.UserInfo.Groups, mcName)) } func (v *fleetResourceValidator) decodeRequestObject(req admission.Request, obj runtime.Object) error { @@ -115,10 +133,18 @@ func createCRDGVK() metav1.GroupVersionKind { } } -func createMemberClusterGVK() metav1.GroupVersionKind { +func createV1alpha1MemberClusterGVK() metav1.GroupVersionKind { return metav1.GroupVersionKind{ Group: fleetv1alpha1.GroupVersion.Group, Version: fleetv1alpha1.GroupVersion.Version, Kind: memberClusterKind, } } + +func createV1beta1MemberClusterGVK() metav1.GroupVersionKind { + return metav1.GroupVersionKind{ + Group: fleetv1beta1.GroupVersion.Group, + Version: fleetv1beta1.GroupVersion.Version, + Kind: memberClusterKind, + } +} diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index ee99349dc..1f2a3a327 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" + fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/cmd/hubagent/options" "go.goms.io/fleet/pkg/webhook/clusterresourceplacement" @@ -50,6 +51,12 @@ const ( memberClusterResourceName = "memberclusters" replicaSetResourceName = "replicasets" podResourceName = "pods" + + crdKind = "CustomResourceDefinition" + mcKind = "MemberCluster" + replicaSetKind = "ReplicaSet" + podKind = "Pod" + crpKind = "ClusterResourcePlacement" ) var ( @@ -129,7 +136,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { Webhooks: []admv1.ValidatingWebhook{ { Name: "fleet.pod.validating", - ClientConfig: w.createClientConfig(corev1.Pod{}), + ClientConfig: w.createClientConfig(podKind), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -150,7 +157,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.clusterresourceplacement.validating", - ClientConfig: w.createClientConfig(fleetv1alpha1.ClusterResourcePlacement{}), + ClientConfig: w.createClientConfig(crpKind), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -172,7 +179,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.replicaset.validating", - ClientConfig: w.createClientConfig(appsv1.ReplicaSet{}), + ClientConfig: w.createClientConfig(replicaSetKind), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -192,7 +199,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.customresourcedefinition.validating", - ClientConfig: w.createClientConfig(v1.CustomResourceDefinition{}), + ClientConfig: w.createClientConfig(crdKind), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -214,7 +221,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.membercluster.validating", - ClientConfig: w.createClientConfig(fleetv1alpha1.MemberCluster{}), + ClientConfig: w.createClientConfig(mcKind), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -226,8 +233,8 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { admv1.Delete, }, Rule: admv1.Rule{ - APIGroups: []string{fleetv1alpha1.GroupVersion.Group}, - APIVersions: []string{fleetv1alpha1.GroupVersion.Version}, + APIGroups: []string{fleetv1alpha1.GroupVersion.Group, fleetv1beta1.GroupVersion.Group}, + APIVersions: []string{fleetv1alpha1.GroupVersion.Version, fleetv1beta1.GroupVersion.Version}, Resources: []string{memberClusterResourceName}, Scope: &clusterScope, }, @@ -264,27 +271,27 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { } // createClientConfig generates the client configuration with either service ref or URL for the argued interface. -func (w *Config) createClientConfig(webhookInterface interface{}) admv1.WebhookClientConfig { +func (w *Config) createClientConfig(kind string) admv1.WebhookClientConfig { serviceRef := admv1.ServiceReference{ Namespace: w.serviceNamespace, Name: FleetWebhookSvcName, Port: pointer.Int32(w.servicePort), } var serviceEndpoint string - switch webhookInterface.(type) { - case corev1.Pod: + switch kind { + case podKind: serviceEndpoint = w.serviceURL + pod.ValidationPath serviceRef.Path = pointer.String(pod.ValidationPath) - case fleetv1alpha1.ClusterResourcePlacement: + case crpKind: serviceEndpoint = w.serviceURL + clusterresourceplacement.ValidationPath serviceRef.Path = pointer.String(clusterresourceplacement.ValidationPath) - case appsv1.ReplicaSet: + case replicaSetKind: serviceEndpoint = w.serviceURL + replicaset.ValidationPath serviceRef.Path = pointer.String(replicaset.ValidationPath) - case v1.CustomResourceDefinition: + case crdKind: serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) - case fleetv1alpha1.MemberCluster: + case mcKind: serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) } diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index ce427a188..33c3fbef2 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -26,7 +26,8 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" workv1alpha1 "sigs.k8s.io/work-api/pkg/apis/v1alpha1" - "go.goms.io/fleet/apis/v1alpha1" + fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/test/e2e/framework" testutils "go.goms.io/fleet/test/e2e/utils" @@ -39,8 +40,8 @@ var ( MemberCluster = framework.NewCluster(memberClusterName, scheme) hubURL string scheme = runtime.NewScheme() - mc *v1alpha1.MemberCluster - imc *v1alpha1.InternalMemberCluster + mc *fleetv1alpha1.MemberCluster + imc *fleetv1alpha1.InternalMemberCluster ctx context.Context // The fleet-system namespace. @@ -73,15 +74,15 @@ var ( sortOption = cmpopts.SortSlices(func(ref1, ref2 metav1.Condition) bool { return ref1.Type < ref2.Type }) imcStatusCmpOptions = []cmp.Option{ - cmpopts.IgnoreTypes(v1alpha1.ResourceUsage{}), + cmpopts.IgnoreTypes(fleetv1alpha1.ResourceUsage{}), cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "ObservedGeneration"), - cmpopts.IgnoreFields(v1alpha1.AgentStatus{}, "LastReceivedHeartbeat"), + cmpopts.IgnoreFields(fleetv1alpha1.AgentStatus{}, "LastReceivedHeartbeat"), sortOption, } mcStatusCmpOptions = []cmp.Option{ cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "ObservedGeneration"), - cmpopts.IgnoreFields(v1alpha1.AgentStatus{}, "LastReceivedHeartbeat"), - cmpopts.IgnoreFields(v1alpha1.ResourceUsage{}, "ObservationTime"), + cmpopts.IgnoreFields(fleetv1alpha1.AgentStatus{}, "LastReceivedHeartbeat"), + cmpopts.IgnoreFields(fleetv1alpha1.ResourceUsage{}, "ObservationTime"), sortOption, } crpStatusCmpOptions = []cmp.Option{ @@ -89,36 +90,36 @@ var ( sortOption, } - imcJoinedAgentStatus = []v1alpha1.AgentStatus{ + imcJoinedAgentStatus = []fleetv1alpha1.AgentStatus{ { - Type: v1alpha1.MemberAgent, + Type: fleetv1alpha1.MemberAgent, Conditions: []metav1.Condition{ { Reason: "InternalMemberClusterHealthy", Status: metav1.ConditionTrue, - Type: string(v1alpha1.AgentHealthy), + Type: string(fleetv1alpha1.AgentHealthy), }, { Reason: "InternalMemberClusterJoined", Status: metav1.ConditionTrue, - Type: string(v1alpha1.AgentJoined), + Type: string(fleetv1alpha1.AgentJoined), }, }, }, } - imcLeftAgentStatus = []v1alpha1.AgentStatus{ + imcLeftAgentStatus = []fleetv1alpha1.AgentStatus{ { - Type: v1alpha1.MemberAgent, + Type: fleetv1alpha1.MemberAgent, Conditions: []metav1.Condition{ { Reason: "InternalMemberClusterHealthy", Status: metav1.ConditionTrue, - Type: string(v1alpha1.AgentHealthy), + Type: string(fleetv1alpha1.AgentHealthy), }, { Reason: "InternalMemberClusterLeft", Status: metav1.ConditionFalse, - Type: string(v1alpha1.AgentJoined), + Type: string(fleetv1alpha1.AgentJoined), }, }, }, @@ -128,12 +129,12 @@ var ( { Reason: "MemberClusterReadyToJoin", Status: metav1.ConditionTrue, - Type: string(v1alpha1.ConditionTypeMemberClusterReadyToJoin), + Type: string(fleetv1alpha1.ConditionTypeMemberClusterReadyToJoin), }, { Reason: "MemberClusterJoined", Status: metav1.ConditionTrue, - Type: string(v1alpha1.ConditionTypeMemberClusterJoined), + Type: string(fleetv1alpha1.ConditionTypeMemberClusterJoined), }, } @@ -141,12 +142,12 @@ var ( { Reason: "MemberClusterNotReadyToJoin", Status: metav1.ConditionFalse, - Type: string(v1alpha1.ConditionTypeMemberClusterReadyToJoin), + Type: string(fleetv1alpha1.ConditionTypeMemberClusterReadyToJoin), }, { Reason: "MemberClusterLeft", Status: metav1.ConditionFalse, - Type: string(v1alpha1.ConditionTypeMemberClusterJoined), + Type: string(fleetv1alpha1.ConditionTypeMemberClusterJoined), }, } @@ -159,7 +160,8 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(v1alpha1.AddToScheme(scheme)) + utilruntime.Must(fleetv1alpha1.AddToScheme(scheme)) + utilruntime.Must(fleetv1beta1.AddToScheme(scheme)) utilruntime.Must(workv1alpha1.AddToScheme(scheme)) utilruntime.Must(apiextensionsv1.AddToScheme(scheme)) } @@ -190,13 +192,13 @@ var _ = BeforeSuite(func() { Kind: "ServiceAccount", Namespace: utils.FleetSystemNamespace, } - mc = &v1alpha1.MemberCluster{ + mc = &fleetv1alpha1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: MemberCluster.ClusterName, }, - Spec: v1alpha1.MemberClusterSpec{ + Spec: fleetv1alpha1.MemberClusterSpec{ Identity: identity, - State: v1alpha1.ClusterStateJoin, + State: fleetv1alpha1.ClusterStateJoin, HeartbeatPeriodSeconds: 60, }, } @@ -205,7 +207,7 @@ var _ = BeforeSuite(func() { }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed(), "Failed to wait for member cluster %s to be created in %s cluster", mc.Name, HubCluster.ClusterName) By("check if internal member cluster created in the hub cluster") - imc = &v1alpha1.InternalMemberCluster{ + imc = &fleetv1alpha1.InternalMemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: MemberCluster.ClusterName, Namespace: memberNamespace.Name, @@ -216,12 +218,12 @@ var _ = BeforeSuite(func() { }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed(), "Failed to wait for internal member cluster %s to be synced in %s cluster", imc.Name, HubCluster.ClusterName) By("check if internal member cluster status is updated to Joined") - wantIMCStatus := v1alpha1.InternalMemberClusterStatus{AgentStatus: imcJoinedAgentStatus} + wantIMCStatus := fleetv1alpha1.InternalMemberClusterStatus{AgentStatus: imcJoinedAgentStatus} testutils.CheckInternalMemberClusterStatus(ctx, *HubCluster, &types.NamespacedName{Name: imc.Name, Namespace: imc.Namespace}, wantIMCStatus, imcStatusCmpOptions) By("check if member cluster status is updated to Joined") Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: imc.Name, Namespace: imc.Namespace}, imc)).Should(Succeed(), "Failed to retrieve internal member cluster %s in %s cluster", imc.Name, HubCluster.ClusterName) - wantMCStatus := v1alpha1.MemberClusterStatus{ + wantMCStatus := fleetv1alpha1.MemberClusterStatus{ AgentStatus: imc.Status.AgentStatus, Conditions: mcJoinedConditions, ResourceUsage: imc.Status.ResourceUsage, @@ -238,16 +240,16 @@ var _ = AfterSuite(func() { By("update member cluster in the hub cluster") Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: mc.Name}, mc)).Should(Succeed(), "Failed to retrieve member cluster %s in %s cluster", mc.Name, HubCluster.ClusterName) - mc.Spec.State = v1alpha1.ClusterStateLeave + mc.Spec.State = fleetv1alpha1.ClusterStateLeave Expect(HubCluster.KubeClient.Update(ctx, mc)).Should(Succeed(), "Failed to update member cluster %s in %s cluster", mc.Name, HubCluster.ClusterName) By("check if internal member cluster status is updated to Left") - wantIMCStatus := v1alpha1.InternalMemberClusterStatus{AgentStatus: imcLeftAgentStatus} + wantIMCStatus := fleetv1alpha1.InternalMemberClusterStatus{AgentStatus: imcLeftAgentStatus} testutils.CheckInternalMemberClusterStatus(ctx, *HubCluster, &types.NamespacedName{Name: imc.Name, Namespace: imc.Namespace}, wantIMCStatus, imcStatusCmpOptions) By("check if member cluster status is updated to Left") Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: imc.Name, Namespace: imc.Namespace}, imc)).Should(Succeed(), "Failed to retrieve internal member cluster %s in %s cluster", imc.Name, HubCluster.ClusterName) - wantMCStatus := v1alpha1.MemberClusterStatus{ + wantMCStatus := fleetv1alpha1.MemberClusterStatus{ AgentStatus: imc.Status.AgentStatus, Conditions: mcLeftConditions, ResourceUsage: imc.Status.ResourceUsage, diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 596576779..8df01d7ef 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -23,6 +23,7 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" + fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/utils" testutils "go.goms.io/fleet/test/e2e/utils" @@ -550,7 +551,7 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() { var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Context("CR validation webhook", func() { - It("should deny CREATE operation on member cluster CR for user not in system:masters group", func() { + It("should deny CREATE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { mc := fleetv1alpha1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-member-cluster", @@ -573,7 +574,30 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) }) - It("should deny UPDATE operation on member cluster CR for user not in system:masters group", func() { + It("should deny CREATE operation on v1beta1 member cluster CR for user not in system:masters group", func() { + mc := fleetv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + Spec: fleetv1beta1.MemberClusterSpec{ + State: fleetv1beta1.ClusterStateJoin, + Identity: rbacv1.Subject{ + Kind: "User", + APIGroup: "", + Name: "test-subject", + Namespace: "fleet-system", + }, + }, + } + + By("expecting denial of operation CREATE of member cluster") + err := HubCluster.ImpersonateKubeClient.Create(ctx, &mc) + var statusErr *k8sErrors.StatusError + Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) + Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) + }) + + It("should deny UPDATE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { var mc fleetv1alpha1.MemberCluster Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: MemberCluster.ClusterName}, &mc)).Should(Succeed()) @@ -587,7 +611,7 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) }) - It("should deny DELETE operation on member cluster CR for user not in system:masters group", func() { + It("should deny DELETE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { mc := fleetv1alpha1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: MemberCluster.ClusterName, From 0b0cf81aa9937c7954a54a91d036c5dcbc350c09 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Tue, 4 Jul 2023 13:36:21 -0700 Subject: [PATCH 2/6] remove webhook handler for v1alpha1 mc --- .../fleetresourcehandler_webhook.go | 42 ++++--------------- pkg/webhook/webhook.go | 34 +++++++-------- test/e2e/webhook_test.go | 35 +++++++++++----- 3 files changed, 46 insertions(+), 65 deletions(-) diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index 74f0017b8..e2616579a 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -17,7 +17,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" - fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/webhook/validation" ) @@ -49,14 +48,8 @@ func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Reque case createCRDGVK(): klog.V(2).InfoS("handling CRD resource", "GVK", createCRDGVK()) response = v.handleCRD(req) - case createV1alpha1MemberClusterGVK(): - gvk := createV1alpha1MemberClusterGVK() - klog.V(2).InfoS("handling Member cluster resource", "GVK", gvk) - response = v.handleMemberCluster(ctx, req, gvk) case createV1beta1MemberClusterGVK(): - gvk := createV1beta1MemberClusterGVK() - klog.V(2).InfoS("handling Member cluster resource", "GVK", gvk) - response = v.handleMemberCluster(ctx, req, gvk) + response = v.handleMemberCluster(ctx, req) default: klog.V(2).InfoS("resource is not monitored by fleet resource validator webhook", "GVK", req.Kind.String()) response = admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify resource with GVK: %s", req.UserInfo.Username, req.UserInfo.Groups, req.Kind.String())) @@ -79,29 +72,16 @@ func (v *fleetResourceValidator) handleCRD(req admission.Request) admission.Resp return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) } -func (v *fleetResourceValidator) handleMemberCluster(ctx context.Context, req admission.Request, gvk metav1.GroupVersionKind) admission.Response { - var mcName string - switch gvk { - case createV1alpha1MemberClusterGVK(): - var mc fleetv1alpha1.MemberCluster - if err := v.decodeRequestObject(req, &mc); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - mcName = mc.Name - case createV1beta1MemberClusterGVK(): - var mc fleetv1beta1.MemberCluster - if err := v.decodeRequestObject(req, &mc); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - mcName = mc.Name - default: - return admission.Denied(fmt.Sprintf("unsupported member cluster group: %s, version: %s & kind: %s", gvk.Group, gvk.Version, gvk.Kind)) +func (v *fleetResourceValidator) handleMemberCluster(ctx context.Context, req admission.Request) admission.Response { + var mc fleetv1beta1.MemberCluster + if err := v.decodeRequestObject(req, &mc); err != nil { + return admission.Errored(http.StatusBadRequest, err) } if !validation.ValidateUserForFleetCR(ctx, v.client, v.whiteListedUsers, req.UserInfo) { - return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify member cluster CR: %s", req.UserInfo.Username, req.UserInfo.Groups, mcName)) + return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify member cluster CR: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) } klog.V(2).InfoS("user in groups is allowed to modify member cluster CR", "user", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify member cluster: %s", req.UserInfo.Username, req.UserInfo.Groups, mcName)) + return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify member cluster: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) } func (v *fleetResourceValidator) decodeRequestObject(req admission.Request, obj runtime.Object) error { @@ -133,14 +113,6 @@ func createCRDGVK() metav1.GroupVersionKind { } } -func createV1alpha1MemberClusterGVK() metav1.GroupVersionKind { - return metav1.GroupVersionKind{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Kind: memberClusterKind, - } -} - func createV1beta1MemberClusterGVK() metav1.GroupVersionKind { return metav1.GroupVersionKind{ Group: fleetv1beta1.GroupVersion.Group, diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 1f2a3a327..11e6a6b6a 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -51,12 +51,6 @@ const ( memberClusterResourceName = "memberclusters" replicaSetResourceName = "replicasets" podResourceName = "pods" - - crdKind = "CustomResourceDefinition" - mcKind = "MemberCluster" - replicaSetKind = "ReplicaSet" - podKind = "Pod" - crpKind = "ClusterResourcePlacement" ) var ( @@ -136,7 +130,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { Webhooks: []admv1.ValidatingWebhook{ { Name: "fleet.pod.validating", - ClientConfig: w.createClientConfig(podKind), + ClientConfig: w.createClientConfig(corev1.Pod{}), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -157,7 +151,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.clusterresourceplacement.validating", - ClientConfig: w.createClientConfig(crpKind), + ClientConfig: w.createClientConfig(fleetv1alpha1.ClusterResourcePlacement{}), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -179,7 +173,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.replicaset.validating", - ClientConfig: w.createClientConfig(replicaSetKind), + ClientConfig: w.createClientConfig(appsv1.ReplicaSet{}), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -199,7 +193,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.customresourcedefinition.validating", - ClientConfig: w.createClientConfig(crdKind), + ClientConfig: w.createClientConfig(v1.CustomResourceDefinition{}), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -221,7 +215,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, { Name: "fleet.membercluster.validating", - ClientConfig: w.createClientConfig(mcKind), + ClientConfig: w.createClientConfig(fleetv1beta1.MemberCluster{}), FailurePolicy: &failPolicy, SideEffects: &sideEffortsNone, AdmissionReviewVersions: admissionReviewVersions, @@ -233,8 +227,8 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { admv1.Delete, }, Rule: admv1.Rule{ - APIGroups: []string{fleetv1alpha1.GroupVersion.Group, fleetv1beta1.GroupVersion.Group}, - APIVersions: []string{fleetv1alpha1.GroupVersion.Version, fleetv1beta1.GroupVersion.Version}, + APIGroups: []string{fleetv1beta1.GroupVersion.Group}, + APIVersions: []string{fleetv1beta1.GroupVersion.Version}, Resources: []string{memberClusterResourceName}, Scope: &clusterScope, }, @@ -271,27 +265,27 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { } // createClientConfig generates the client configuration with either service ref or URL for the argued interface. -func (w *Config) createClientConfig(kind string) admv1.WebhookClientConfig { +func (w *Config) createClientConfig(webhookInterface interface{}) admv1.WebhookClientConfig { serviceRef := admv1.ServiceReference{ Namespace: w.serviceNamespace, Name: FleetWebhookSvcName, Port: pointer.Int32(w.servicePort), } var serviceEndpoint string - switch kind { - case podKind: + switch webhookInterface.(type) { + case corev1.Pod: serviceEndpoint = w.serviceURL + pod.ValidationPath serviceRef.Path = pointer.String(pod.ValidationPath) - case crpKind: + case fleetv1alpha1.ClusterResourcePlacement: serviceEndpoint = w.serviceURL + clusterresourceplacement.ValidationPath serviceRef.Path = pointer.String(clusterresourceplacement.ValidationPath) - case replicaSetKind: + case appsv1.ReplicaSet: serviceEndpoint = w.serviceURL + replicaset.ValidationPath serviceRef.Path = pointer.String(replicaset.ValidationPath) - case crdKind: + case v1.CustomResourceDefinition: serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) - case mcKind: + case fleetv1beta1.MemberCluster: serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) } diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 8df01d7ef..31415b482 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -551,13 +551,13 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() { var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Context("CR validation webhook", func() { - It("should deny CREATE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { - mc := fleetv1alpha1.MemberCluster{ + It("should deny CREATE operation member cluster CR for user not in system:masters group", func() { + mc := fleetv1beta1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-member-cluster", }, - Spec: fleetv1alpha1.MemberClusterSpec{ - State: fleetv1alpha1.ClusterStateJoin, + Spec: fleetv1beta1.MemberClusterSpec{ + State: fleetv1beta1.ClusterStateJoin, Identity: rbacv1.Subject{ Kind: "User", APIGroup: "", @@ -597,12 +597,27 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) }) - It("should deny UPDATE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { - var mc fleetv1alpha1.MemberCluster - Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: MemberCluster.ClusterName}, &mc)).Should(Succeed()) + It("should deny UPDATE operation on member cluster CR for user not in system:masters group", func() { + mc := fleetv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + Spec: fleetv1beta1.MemberClusterSpec{ + State: fleetv1beta1.ClusterStateJoin, + Identity: rbacv1.Subject{ + Kind: "User", + APIGroup: "", + Name: "test-subject", + Namespace: "fleet-system", + }, + }, + } + Eventually(func() error { + return HubCluster.KubeClient.Update(ctx, &mc) + }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed()) By("update member cluster spec") - mc.Spec.State = fleetv1alpha1.ClusterStateLeave + mc.Spec.State = fleetv1beta1.ClusterStateLeave By("expecting denial of operation UPDATE of member cluster") err := HubCluster.ImpersonateKubeClient.Update(ctx, &mc) @@ -611,8 +626,8 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) }) - It("should deny DELETE operation on v1alpha1 member cluster CR for user not in system:masters group", func() { - mc := fleetv1alpha1.MemberCluster{ + It("should deny DELETE operation on member cluster CR for user not in system:masters group", func() { + mc := fleetv1beta1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: MemberCluster.ClusterName, }, From 2d1fd3dcc5caf6e7169ec4e23490ada691063cc4 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Tue, 4 Jul 2023 15:25:25 -0700 Subject: [PATCH 3/6] fix e2e --- ...> placement.azure.com_memberclusters.yaml} | 4 +-- test/e2e/e2e_test.go | 8 ++--- test/e2e/utils/helper.go | 33 +++++++++++++++++-- test/e2e/webhook_test.go | 24 ++++---------- 4 files changed, 43 insertions(+), 26 deletions(-) rename charts/hub-agent/templates/crds/{placement.karavel.io_memberclusters.yaml => placement.azure.com_memberclusters.yaml} (99%) diff --git a/charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml b/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml similarity index 99% rename from charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml rename to charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml index fa1305ee1..267f3a7e9 100644 --- a/charts/hub-agent/templates/crds/placement.karavel.io_memberclusters.yaml +++ b/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml @@ -4,9 +4,9 @@ kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.11.4 - name: memberclusters.placement.karavel.io + name: memberclusters.placement.azure.com spec: - group: placement.karavel.io + group: placement.azure.com names: categories: - fleet diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 33c3fbef2..00b4bcf8a 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -230,13 +230,13 @@ var _ = BeforeSuite(func() { } testutils.CheckMemberClusterStatus(ctx, *HubCluster, &types.NamespacedName{Name: mc.Name}, wantMCStatus, mcStatusCmpOptions) - By("create cluster role binding and cluster roles for webhook e2e") - testutils.CreateClusterRoleAndClusterRoleBindingsForWebHookE2E(ctx, HubCluster) + By("create member cluster, cluster role binding, cluster roles for webhook e2e") + testutils.CreateResourcesForWebHookE2E(ctx, HubCluster) }) var _ = AfterSuite(func() { - By("delete cluster role binding and cluster roles for webhook e2e") - testutils.DeleteClusterRoleAndClusterRoleBindingForWebHookE2E(ctx, HubCluster) + By("delete member cluster, cluster role binding and cluster roles for webhook e2e") + testutils.DeleteResourcesForWebHookE2E(ctx, HubCluster) By("update member cluster in the hub cluster") Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: mc.Name}, mc)).Should(Succeed(), "Failed to retrieve member cluster %s in %s cluster", mc.Name, HubCluster.ClusterName) diff --git a/test/e2e/utils/helper.go b/test/e2e/utils/helper.go index 1368cca63..f6d8612bb 100644 --- a/test/e2e/utils/helper.go +++ b/test/e2e/utils/helper.go @@ -27,6 +27,7 @@ import ( "k8s.io/klog/v2" workapi "sigs.k8s.io/work-api/pkg/apis/v1alpha1" + fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/test/e2e/framework" @@ -196,7 +197,26 @@ func GenerateCRDObjectFromFile(cluster framework.Cluster, fs embed.FS, filepath return obj, gvk, mapping.Resource } -func CreateClusterRoleAndClusterRoleBindingsForWebHookE2E(ctx context.Context, hubCluster *framework.Cluster) { +func CreateResourcesForWebHookE2E(ctx context.Context, hubCluster *framework.Cluster) { + mc := fleetv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + Spec: fleetv1beta1.MemberClusterSpec{ + State: fleetv1beta1.ClusterStateJoin, + Identity: rbacv1.Subject{ + Kind: "User", + APIGroup: "", + Name: "test-subject", + Namespace: "fleet-system", + }, + }, + } + + gomega.Eventually(func() error { + return hubCluster.KubeClient.Create(ctx, &mc) + }, PollTimeout, PollInterval).Should(gomega.Succeed(), "failed to create member cluster %s", mc.Name) + cr := rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: crdClusterRole, @@ -241,7 +261,7 @@ func CreateClusterRoleAndClusterRoleBindingsForWebHookE2E(ctx context.Context, h }, Rules: []rbacv1.PolicyRule{ { - APIGroups: []string{fleetv1alpha1.GroupVersion.Group}, + APIGroups: []string{fleetv1beta1.GroupVersion.Group}, Verbs: []string{"*"}, Resources: []string{"*"}, }, @@ -273,7 +293,7 @@ func CreateClusterRoleAndClusterRoleBindingsForWebHookE2E(ctx context.Context, h }, PollTimeout, PollInterval).Should(gomega.Succeed(), "failed to create cluster role binding %s to modify CRDs", crb.Name) } -func DeleteClusterRoleAndClusterRoleBindingForWebHookE2E(ctx context.Context, hubCluster *framework.Cluster) { +func DeleteResourcesForWebHookE2E(ctx context.Context, hubCluster *framework.Cluster) { crb := rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: crClusterRoleBinding, @@ -301,4 +321,11 @@ func DeleteClusterRoleAndClusterRoleBindingForWebHookE2E(ctx context.Context, hu }, } gomega.Expect(hubCluster.KubeClient.Delete(ctx, &cr)).Should(gomega.Succeed()) + + mc := fleetv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + } + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &mc)).Should(gomega.Succeed()) } diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 31415b482..0f1b57e72 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -598,20 +598,10 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { }) It("should deny UPDATE operation on member cluster CR for user not in system:masters group", func() { - mc := fleetv1beta1.MemberCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-member-cluster", - }, - Spec: fleetv1beta1.MemberClusterSpec{ - State: fleetv1beta1.ClusterStateJoin, - Identity: rbacv1.Subject{ - Kind: "User", - APIGroup: "", - Name: "test-subject", - Namespace: "fleet-system", - }, - }, - } + var mc fleetv1beta1.MemberCluster + Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "test-member-cluster"}, &mc)).Should(Succeed()) + + mc.Spec.State = fleetv1beta1.ClusterStateLeave Eventually(func() error { return HubCluster.KubeClient.Update(ctx, &mc) }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed()) @@ -629,7 +619,7 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { It("should deny DELETE operation on member cluster CR for user not in system:masters group", func() { mc := fleetv1beta1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: MemberCluster.ClusterName, + Name: "test-member-cluster", }, } @@ -641,8 +631,8 @@ var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { }) It("should allow update operation on member cluster CR for user in system:masters group", func() { - var mc fleetv1alpha1.MemberCluster - Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: MemberCluster.ClusterName}, &mc)).Should(Succeed()) + var mc fleetv1beta1.MemberCluster + Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "test-member-cluster"}, &mc)).Should(Succeed()) By("update labels in CRD") labels := make(map[string]string) From c9f6596677e67cab2f4efa876b455b1cc0bb82de Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Tue, 4 Jul 2023 16:34:07 -0700 Subject: [PATCH 4/6] add symlink instead --- .../placement.azure.com_memberclusters.yaml | 306 +----------------- 1 file changed, 1 insertion(+), 305 deletions(-) mode change 100644 => 120000 charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml diff --git a/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml b/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml deleted file mode 100644 index 267f3a7e9..000000000 --- a/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml +++ /dev/null @@ -1,305 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.11.4 - name: memberclusters.placement.azure.com -spec: - group: placement.azure.com - names: - categories: - - fleet - kind: MemberCluster - listKind: MemberClusterList - plural: memberclusters - shortNames: - - mc - singular: membercluster - scope: Cluster - versions: - - additionalPrinterColumns: - - jsonPath: .status.conditions[?(@.type=="Joined")].status - name: Joined - type: string - - jsonPath: .metadata.creationTimestamp - name: Age - type: date - name: v1beta1 - schema: - openAPIV3Schema: - description: MemberCluster is a resource created in the hub cluster to represent - a member cluster within a fleet. - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - description: The desired state of MemberCluster. - properties: - heartbeatPeriodSeconds: - default: 60 - description: 'How often (in seconds) for the member cluster to send - a heartbeat to the hub cluster. Default: 60 seconds. Min: 1 second. - Max: 10 minutes.' - format: int32 - maximum: 600 - minimum: 1 - type: integer - identity: - description: The identity used by the member cluster to access the - hub cluster. The hub agents deployed on the hub cluster will automatically - grant the minimal required permissions to this identity for the - member agents deployed on the member cluster to access the hub cluster. - properties: - apiGroup: - description: APIGroup holds the API group of the referenced subject. - Defaults to "" for ServiceAccount subjects. Defaults to "rbac.authorization.k8s.io" - for User and Group subjects. - type: string - kind: - description: Kind of object being referenced. Values defined by - this API group are "User", "Group", and "ServiceAccount". If - the Authorizer does not recognized the kind value, the Authorizer - should report an error. - type: string - name: - description: Name of the object being referenced. - type: string - namespace: - description: Namespace of the referenced object. If the object - kind is non-namespace, such as "User" or "Group", and this value - is not empty the Authorizer should report an error. - type: string - required: - - kind - - name - type: object - x-kubernetes-map-type: atomic - state: - description: 'The desired state of the member cluster. Possible values: - Join, Leave.' - type: string - required: - - identity - - state - type: object - status: - description: The observed status of MemberCluster. - properties: - agentStatus: - description: AgentStatus is an array of current observed status, each - corresponding to one member agent running in the member cluster. - items: - description: AgentStatus defines the observed status of the member - agent of the given type. - properties: - conditions: - description: Conditions is an array of current observed conditions - for the member agent. - items: - description: "Condition contains details for one aspect of - the current state of this API Resource. --- This struct - is intended for direct use as an array at the field path - .status.conditions. For example, \n type FooStatus struct{ - // Represents the observations of a foo's current state. - // Known .status.conditions.type are: \"Available\", \"Progressing\", - and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge - // +listType=map // +listMapKey=type Conditions []metav1.Condition - `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" - protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields - }" - properties: - lastTransitionTime: - description: lastTransitionTime is the last time the condition - transitioned from one status to another. This should - be when the underlying condition changed. If that is - not known, then using the time when the API field changed - is acceptable. - format: date-time - type: string - message: - description: message is a human readable message indicating - details about the transition. This may be an empty string. - maxLength: 32768 - type: string - observedGeneration: - description: observedGeneration represents the .metadata.generation - that the condition was set based upon. For instance, - if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration - is 9, the condition is out of date with respect to the - current state of the instance. - format: int64 - minimum: 0 - type: integer - reason: - description: reason contains a programmatic identifier - indicating the reason for the condition's last transition. - Producers of specific condition types may define expected - values and meanings for this field, and whether the - values are considered a guaranteed API. The value should - be a CamelCase string. This field may not be empty. - maxLength: 1024 - minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ - type: string - status: - description: status of the condition, one of True, False, - Unknown. - enum: - - "True" - - "False" - - Unknown - type: string - type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - --- Many .condition.type values are consistent across - resources like Available, but because arbitrary conditions - can be useful (see .node.status.conditions), the ability - to deconflict is important. The regex it matches is - (dns1123SubdomainFmt/)?(qualifiedNameFmt) - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ - type: string - required: - - lastTransitionTime - - message - - reason - - status - - type - type: object - type: array - x-kubernetes-list-map-keys: - - type - x-kubernetes-list-type: map - lastReceivedHeartbeat: - description: Last time we received a heartbeat from the member - agent. - format: date-time - type: string - type: - description: Type of the member agent. - type: string - required: - - type - type: object - type: array - conditions: - description: Conditions is an array of current observed conditions - for the member cluster. - items: - description: "Condition contains details for one aspect of the current - state of this API Resource. --- This struct is intended for direct - use as an array at the field path .status.conditions. For example, - \n type FooStatus struct{ // Represents the observations of a - foo's current state. // Known .status.conditions.type are: \"Available\", - \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge - // +listType=map // +listMapKey=type Conditions []metav1.Condition - `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" - protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" - properties: - lastTransitionTime: - description: lastTransitionTime is the last time the condition - transitioned from one status to another. This should be when - the underlying condition changed. If that is not known, then - using the time when the API field changed is acceptable. - format: date-time - type: string - message: - description: message is a human readable message indicating - details about the transition. This may be an empty string. - maxLength: 32768 - type: string - observedGeneration: - description: observedGeneration represents the .metadata.generation - that the condition was set based upon. For instance, if .metadata.generation - is currently 12, but the .status.conditions[x].observedGeneration - is 9, the condition is out of date with respect to the current - state of the instance. - format: int64 - minimum: 0 - type: integer - reason: - description: reason contains a programmatic identifier indicating - the reason for the condition's last transition. Producers - of specific condition types may define expected values and - meanings for this field, and whether the values are considered - a guaranteed API. The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 - minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ - type: string - status: - description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown - type: string - type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - --- Many .condition.type values are consistent across resources - like Available, but because arbitrary conditions can be useful - (see .node.status.conditions), the ability to deconflict is - important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ - type: string - required: - - lastTransitionTime - - message - - reason - - status - - type - type: object - type: array - x-kubernetes-list-map-keys: - - type - x-kubernetes-list-type: map - resourceUsage: - description: The current observed resource usage of the member cluster. - It is copied from the corresponding InternalMemberCluster object. - properties: - allocatable: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: Allocatable represents the total resources of all - the nodes on a member cluster that are available for scheduling. - type: object - capacity: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: Capacity represents the total resource capacity of - all the nodes on a member cluster. - type: object - observationTime: - description: When the resource usage is observed. - format: date-time - type: string - type: object - type: object - required: - - spec - type: object - served: true - storage: true - subresources: - status: {} diff --git a/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml b/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml new file mode 120000 index 000000000..d4511d78f --- /dev/null +++ b/charts/hub-agent/templates/crds/placement.azure.com_memberclusters.yaml @@ -0,0 +1 @@ +../../../../config/crd/bases/placement.azure.com_memberclusters.yaml \ No newline at end of file From f9ca5491999581de9fd8d8a52be98109d35f53a9 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Tue, 4 Jul 2023 16:52:21 -0700 Subject: [PATCH 5/6] revert log removal --- pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index e2616579a..634dd6710 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -49,6 +49,7 @@ func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Reque klog.V(2).InfoS("handling CRD resource", "GVK", createCRDGVK()) response = v.handleCRD(req) case createV1beta1MemberClusterGVK(): + klog.V(2).InfoS("handling Member cluster resource", "GVK", createV1beta1MemberClusterGVK()) response = v.handleMemberCluster(ctx, req) default: klog.V(2).InfoS("resource is not monitored by fleet resource validator webhook", "GVK", req.Kind.String()) From c9dbaff55ea1e01e38de6f61e45cb73c716baf24 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Tue, 4 Jul 2023 16:58:10 -0700 Subject: [PATCH 6/6] fail logs for Expect --- test/e2e/utils/helper.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/e2e/utils/helper.go b/test/e2e/utils/helper.go index f6d8612bb..41023d885 100644 --- a/test/e2e/utils/helper.go +++ b/test/e2e/utils/helper.go @@ -198,6 +198,7 @@ func GenerateCRDObjectFromFile(cluster framework.Cluster, fs embed.FS, filepath } func CreateResourcesForWebHookE2E(ctx context.Context, hubCluster *framework.Cluster) { + // create v1beta1 member cluster CR mc := fleetv1beta1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-member-cluster", @@ -299,33 +300,34 @@ func DeleteResourcesForWebHookE2E(ctx context.Context, hubCluster *framework.Clu Name: crClusterRoleBinding, }, } - gomega.Expect(hubCluster.KubeClient.Delete(ctx, &crb)).Should(gomega.Succeed()) + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &crb)).Should(gomega.Succeed(), "failed to delete CR cluster role binding %s", crb.Name) cr := rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: crClusterRole, }, } - gomega.Expect(hubCluster.KubeClient.Delete(ctx, &cr)).Should(gomega.Succeed()) + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &cr)).Should(gomega.Succeed(), "failed to delete CR cluster role %s", cr.Name) crb = rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: crdClusterRoleBinding, }, } - gomega.Expect(hubCluster.KubeClient.Delete(ctx, &crb)).Should(gomega.Succeed()) + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &crb)).Should(gomega.Succeed(), "failed to delete CRD cluster role binding %s", crb.Name) cr = rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: crdClusterRole, }, } - gomega.Expect(hubCluster.KubeClient.Delete(ctx, &cr)).Should(gomega.Succeed()) + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &cr)).Should(gomega.Succeed(), "failed to delete CRD cluster role %s", cr.Name) + // delete v1beta1 member cluster CR mc := fleetv1beta1.MemberCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-member-cluster", }, } - gomega.Expect(hubCluster.KubeClient.Delete(ctx, &mc)).Should(gomega.Succeed()) + gomega.Expect(hubCluster.KubeClient.Delete(ctx, &mc)).Should(gomega.Succeed(), "failed to delete member cluster %s", mc.Name) }