From bec60e88568c9015b23975f3970eaa5b4333ae28 Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Tue, 4 Aug 2020 18:37:26 +0800 Subject: [PATCH 1/6] Implement the intersection methods --- .../bases/bulward.io_organizationroles.yaml | 216 ++++++++++++ .../core/v1alpha1/organizationrole_types.go | 193 +++++++++++ .../core/v1alpha1/zz_generated.deepcopy.go | 126 +++++++ .../organizationrole_controller.go | 88 +++++ pkg/utils/intersection/intersection.go | 144 ++++++++ pkg/utils/intersection/intersection_test.go | 328 ++++++++++++++++++ 6 files changed, 1095 insertions(+) create mode 100644 config/manager/crd/bases/bulward.io_organizationroles.yaml create mode 100644 pkg/apis/core/v1alpha1/organizationrole_types.go create mode 100644 pkg/manager/internal/controllers/organizationrole_controller.go create mode 100644 pkg/utils/intersection/intersection.go create mode 100644 pkg/utils/intersection/intersection_test.go diff --git a/config/manager/crd/bases/bulward.io_organizationroles.yaml b/config/manager/crd/bases/bulward.io_organizationroles.yaml new file mode 100644 index 0000000..1f648b2 --- /dev/null +++ b/config/manager/crd/bases/bulward.io_organizationroles.yaml @@ -0,0 +1,216 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.2.9 + creationTimestamp: null + name: organizationroles.bulward.io +spec: + group: bulward.io + names: + kind: OrganizationRole + listKind: OrganizationRoleList + plural: organizationroles + singular: organizationrole + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.metadata.displayName + name: Display Name + type: string + - jsonPath: .status.phase + name: Status + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: OrganizationRole is internal representation for organization-scoped + Role in Bulward. + 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: OrganizationRoleSpec describes the desired state of OrganizationRole. + properties: + rules: + description: Rules defines the Role that this OrganizationRole refers + to. + items: + description: PolicyRule holds information that describes a policy + rule, but does not contain information about who the rule applies + to or which namespace the rule applies to. + properties: + apiGroups: + description: APIGroups is the name of the APIGroup that contains + the resources. If multiple API groups are specified, any + action requested against one of the enumerated resources in + any API group will be allowed. + items: + type: string + type: array + nonResourceURLs: + description: NonResourceURLs is a set of partial urls that a + user should have access to. *s are allowed, but only as the + full, final step in the path Since non-resource URLs are not + namespaced, this field is only applicable for ClusterRoles + referenced from a ClusterRoleBinding. Rules can either apply + to API resources (such as "pods" or "secrets") or non-resource + URL paths (such as "/api"), but not both. + items: + type: string + type: array + resourceNames: + description: ResourceNames is an optional white list of names + that the rule applies to. An empty set means that everything + is allowed. + items: + type: string + type: array + resources: + description: Resources is a list of resources this rule applies + to. ResourceAll represents all resources. + items: + type: string + type: array + verbs: + description: Verbs is a list of Verbs that apply to ALL the + ResourceKinds and AttributeRestrictions contained in this + rule. VerbAll represents all kinds. + items: + type: string + type: array + required: + - verbs + type: object + type: array + required: + - rules + type: object + status: + description: OrganizationRoleStatus represents the observed state of OrganizationRole. + properties: + acceptedRules: + description: AcceptedRules contains the rules that accepted by Bulward. + items: + description: PolicyRule holds information that describes a policy + rule, but does not contain information about who the rule applies + to or which namespace the rule applies to. + properties: + apiGroups: + description: APIGroups is the name of the APIGroup that contains + the resources. If multiple API groups are specified, any + action requested against one of the enumerated resources in + any API group will be allowed. + items: + type: string + type: array + nonResourceURLs: + description: NonResourceURLs is a set of partial urls that a + user should have access to. *s are allowed, but only as the + full, final step in the path Since non-resource URLs are not + namespaced, this field is only applicable for ClusterRoles + referenced from a ClusterRoleBinding. Rules can either apply + to API resources (such as "pods" or "secrets") or non-resource + URL paths (such as "/api"), but not both. + items: + type: string + type: array + resourceNames: + description: ResourceNames is an optional white list of names + that the rule applies to. An empty set means that everything + is allowed. + items: + type: string + type: array + resources: + description: Resources is a list of resources this rule applies + to. ResourceAll represents all resources. + items: + type: string + type: array + verbs: + description: Verbs is a list of Verbs that apply to ALL the + ResourceKinds and AttributeRestrictions contained in this + rule. VerbAll represents all kinds. + items: + type: string + type: array + required: + - verbs + type: object + type: array + conditions: + description: Conditions represents the latest available observations + of a OrganizationRole's current state. + items: + description: OrganizationRoleCondition contains details for the + current condition of this OrganizationRole. + properties: + lastTransitionTime: + description: LastTransitionTime is the last time the condition + transits from one status to another. + format: date-time + type: string + message: + description: Message is the human readable message indicating + details about last transition. + type: string + reason: + description: Reason is the (brief) reason for the condition's + last transition. + type: string + status: + description: Status is the status of the condition, one of ('True', + 'False', 'Unknown'). + type: string + type: + description: Type is the type of the OrganizationRole condition, + currently ('Ready'). + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this OrganizationRole by the controller. + format: int64 + type: integer + phase: + description: DEPRECATED. Phase represents the current lifecycle state + of this object. Consider this field DEPRECATED, it will be removed + as soon as there is a mechanism to map conditions to strings when + printing the property. This is only for display purpose, for everything + else use conditions. + type: string + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/pkg/apis/core/v1alpha1/organizationrole_types.go b/pkg/apis/core/v1alpha1/organizationrole_types.go new file mode 100644 index 0000000..a4dc698 --- /dev/null +++ b/pkg/apis/core/v1alpha1/organizationrole_types.go @@ -0,0 +1,193 @@ +/* +Copyright 2020 The Bulward Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// OrganizationRoleSpec describes the desired state of OrganizationRole. +type OrganizationRoleSpec struct { + // Rules defines the Role that this OrganizationRole refers to. + Rules []rbacv1.PolicyRule `json:"rules"` +} + +// OrganizationRoleStatus represents the observed state of OrganizationRole. +type OrganizationRoleStatus struct { + // ObservedGeneration is the most recent generation observed for this OrganizationRole by the controller. + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // Conditions represents the latest available observations of a OrganizationRole's current state. + Conditions []OrganizationRoleCondition `json:"conditions,omitempty"` + // DEPRECATED. + // Phase represents the current lifecycle state of this object. + // Consider this field DEPRECATED, it will be removed as soon as there + // is a mechanism to map conditions to strings when printing the property. + // This is only for display purpose, for everything else use conditions. + Phase OrganizationRolePhaseType `json:"phase,omitempty"` + // AcceptedRules contains the rules that accepted by Bulward. + AcceptedRules []rbacv1.PolicyRule `json:"acceptedRules,omitempty"` +} + +// OrganizationRolePhaseType represents all conditions as a single string for printing by using kubectl commands. +// +kubebuilder:validation:Ready;NotReady;Unknown;Terminating +type OrganizationRolePhaseType string + +// Values of OrganizationRolePhaseType. +const ( + OrganizationRolePhaseReady OrganizationRolePhaseType = "Ready" + OrganizationRolePhaseNotReady OrganizationRolePhaseType = "NotReady" + OrganizationRolePhaseUnknown OrganizationRolePhaseType = "Unknown" + OrganizationRolePhaseTerminating OrganizationRolePhaseType = "Terminating" +) + +const ( + OrganizationRoleTerminatingReason = "Deleting" +) + +// updatePhase updates the phase property based on the current conditions. +// this method should be called every time the conditions are updated. +func (s *OrganizationRoleStatus) updatePhase() { + for _, condition := range s.Conditions { + if condition.Type != OrganizationRoleReady { + continue + } + + switch condition.Status { + case ConditionTrue: + s.Phase = OrganizationRolePhaseReady + case ConditionFalse: + if condition.Reason == OrganizationRoleTerminatingReason { + s.Phase = OrganizationRolePhaseTerminating + } else { + s.Phase = OrganizationRolePhaseNotReady + } + case ConditionUnknown: + s.Phase = OrganizationRolePhaseUnknown + } + return + } + + s.Phase = OrganizationRolePhaseUnknown +} + +// OrganizationRoleConditionType represents a OrganizationRoleCondition value. +// +kubebuilder:validation:Ready +type OrganizationRoleConditionType string + +const ( + // OrganizationRoleReady represents a OrganizationRole condition is in ready state. + OrganizationRoleReady OrganizationRoleConditionType = "Ready" +) + +// OrganizationRoleCondition contains details for the current condition of this OrganizationRole. +type OrganizationRoleCondition struct { + // Type is the type of the OrganizationRole condition, currently ('Ready'). + Type OrganizationRoleConditionType `json:"type"` + // Status is the status of the condition, one of ('True', 'False', 'Unknown'). + Status ConditionStatus `json:"status"` + // LastTransitionTime is the last time the condition transits from one status to another. + LastTransitionTime metav1.Time `json:"lastTransitionTime"` + // Reason is the (brief) reason for the condition's last transition. + Reason string `json:"reason"` + // Message is the human readable message indicating details about last transition. + Message string `json:"message"` +} + +// GetCondition returns the Condition of the given condition type, if it exists. +func (s *OrganizationRoleStatus) GetCondition(t OrganizationRoleConditionType) (condition OrganizationRoleCondition, exists bool) { + for _, cond := range s.Conditions { + if cond.Type == t { + condition = cond + exists = true + return + } + } + return +} + +// SetCondition replaces or adds the given condition. +func (s *OrganizationRoleStatus) SetCondition(condition OrganizationRoleCondition) { + defer s.updatePhase() + + if condition.LastTransitionTime.IsZero() { + condition.LastTransitionTime = metav1.Now() + } + + for i := range s.Conditions { + if s.Conditions[i].Type == condition.Type { + + // Only update the LastTransitionTime when the Status is changed. + if s.Conditions[i].Status != condition.Status { + s.Conditions[i].LastTransitionTime = condition.LastTransitionTime + } + + s.Conditions[i].Status = condition.Status + s.Conditions[i].Reason = condition.Reason + s.Conditions[i].Message = condition.Message + + return + } + } + + s.Conditions = append(s.Conditions, condition) +} + +// OrganizationRole is internal representation for organization-scoped Role in Bulward. +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="Display Name",type="string",JSONPath=".spec.metadata.displayName" +// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" +type OrganizationRole struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec OrganizationRoleSpec `json:"spec,omitempty"` + Status OrganizationRoleStatus `json:"status,omitempty"` +} + +// IsReady returns if the OrganizationRole is ready. +func (s *OrganizationRole) IsReady() bool { + if !s.DeletionTimestamp.IsZero() { + return false + } + + if s.Generation != s.Status.ObservedGeneration { + return false + } + + for _, condition := range s.Status.Conditions { + if condition.Type == OrganizationRoleReady && + condition.Status == ConditionTrue { + return true + } + } + return false +} + +// OrganizationRoleList contains a list of OrganizationRole. +// +kubebuilder:object:root=true +type OrganizationRoleList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []OrganizationRole `json:"items"` +} + +func init() { + SchemeBuilder.Register(&OrganizationRole{}, &OrganizationRoleList{}) +} diff --git a/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go index ac4141d..216c49c 100644 --- a/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go @@ -41,6 +41,132 @@ func (in *ObjectReference) DeepCopy() *ObjectReference { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationRole) DeepCopyInto(out *OrganizationRole) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationRole. +func (in *OrganizationRole) DeepCopy() *OrganizationRole { + if in == nil { + return nil + } + out := new(OrganizationRole) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OrganizationRole) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationRoleCondition) DeepCopyInto(out *OrganizationRoleCondition) { + *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationRoleCondition. +func (in *OrganizationRoleCondition) DeepCopy() *OrganizationRoleCondition { + if in == nil { + return nil + } + out := new(OrganizationRoleCondition) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationRoleList) DeepCopyInto(out *OrganizationRoleList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]OrganizationRole, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationRoleList. +func (in *OrganizationRoleList) DeepCopy() *OrganizationRoleList { + if in == nil { + return nil + } + out := new(OrganizationRoleList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OrganizationRoleList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationRoleSpec) DeepCopyInto(out *OrganizationRoleSpec) { + *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]v1.PolicyRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationRoleSpec. +func (in *OrganizationRoleSpec) DeepCopy() *OrganizationRoleSpec { + if in == nil { + return nil + } + out := new(OrganizationRoleSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationRoleStatus) DeepCopyInto(out *OrganizationRoleStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]OrganizationRoleCondition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.AcceptedRules != nil { + in, out := &in.AcceptedRules, &out.AcceptedRules + *out = make([]v1.PolicyRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationRoleStatus. +func (in *OrganizationRoleStatus) DeepCopy() *OrganizationRoleStatus { + if in == nil { + return nil + } + out := new(OrganizationRoleStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OrganizationRoleTemplate) DeepCopyInto(out *OrganizationRoleTemplate) { *out = *in diff --git a/pkg/manager/internal/controllers/organizationrole_controller.go b/pkg/manager/internal/controllers/organizationrole_controller.go new file mode 100644 index 0000000..b63959c --- /dev/null +++ b/pkg/manager/internal/controllers/organizationrole_controller.go @@ -0,0 +1,88 @@ +/* +Copyright 2020 The Bulward Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "k8c.io/utils/pkg/util" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + corev1alpha1 "k8c.io/bulward/pkg/apis/core/v1alpha1" +) + +type OrganizationRoleReconciler struct { + client.Client + Log logr.Logger + Scheme *runtime.Scheme +} + +func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { + ctx := context.Background() + log := r.Log.WithValues("organizationRole", req.NamespacedName) + + organizationRole := &corev1alpha1.OrganizationRole{} + if err := r.Get(ctx, req.NamespacedName, organizationRole); err != nil { + if errors.IsNotFound(err) { + return ctrl.Result{}, nil + } + } + + if !organizationRole.GetDeletionTimestamp().IsZero() { + if err := r.handleDeletion(ctx, log, organizationRole); err != nil { + return ctrl.Result{}, fmt.Errorf("handling deletion: %w", err) + } + return ctrl.Result{}, nil + } + + if util.AddFinalizer(organizationRole, metav1.FinalizerDeleteDependents) { + if err := r.Client.Update(ctx, organizationRole); err != nil { + return ctrl.Result{}, fmt.Errorf("updating OrganizationRole finalizers: %w", err) + } + } + + return ctrl.Result{}, nil +} + +func (r *OrganizationRoleReconciler) handleDeletion(ctx context.Context, log logr.Logger, organizationRole *corev1alpha1.OrganizationRole) error { + // Update the OrganizationRole Status to Terminating. + readyCondition, _ := organizationRole.Status.GetCondition(corev1alpha1.OrganizationRoleReady) + if readyCondition.Status != corev1alpha1.ConditionFalse || + readyCondition.Status == corev1alpha1.ConditionFalse && readyCondition.Reason != corev1alpha1.OrganizationRoleTerminatingReason { + organizationRole.Status.ObservedGeneration = organizationRole.Generation + organizationRole.Status.SetCondition(corev1alpha1.OrganizationRoleCondition{ + Type: corev1alpha1.OrganizationRoleReady, + Status: corev1alpha1.ConditionFalse, + Reason: corev1alpha1.OrganizationRoleTerminatingReason, + Message: "OrganizationRole is being terminated", + }) + if err := r.Status().Update(ctx, organizationRole); err != nil { + return fmt.Errorf("updating OrganizationRole status: %w", err) + } + } + return nil +} + +// +// func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole) error { +// } diff --git a/pkg/utils/intersection/intersection.go b/pkg/utils/intersection/intersection.go new file mode 100644 index 0000000..4d0d939 --- /dev/null +++ b/pkg/utils/intersection/intersection.go @@ -0,0 +1,144 @@ +/* +Copyright 2020 The Bulward Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package intersection + +import ( + rbacv1 "k8s.io/api/rbac/v1" +) + +func IntersectPolicyRules(rules1, rules2 []rbacv1.PolicyRule) []rbacv1.PolicyRule { + var newRules []rbacv1.PolicyRule + for _, r1 := range rules1 { + for _, r2 := range rules2 { + rule := intersectPolicyRule(&r1, &r2) + if rule != nil { + newRules = append(newRules, *rule) + } + } + } + return newRules +} + +func intersectPolicyRule(rule1, rule2 *rbacv1.PolicyRule) *rbacv1.PolicyRule { + verbs := intersectVerbs(rule1.Verbs, rule2.Verbs) + if len(verbs) == 0 { + return nil + } + policyRule := &rbacv1.PolicyRule{ + Verbs: verbs, + } + apiGroups := intersectAPIGroups(rule1.APIGroups, rule2.APIGroups) + resources := intersectResources(rule1.Resources, rule2.Resources) + resourceNames := intersectResourceNames(rule1.ResourceNames, rule2.ResourceNames) + nonResourceURLs := intersectNonResourceURLs(rule1.NonResourceURLs, rule2.NonResourceURLs) + var isValid bool + if len(apiGroups) != 0 && len(resources) != 0 { + // This rule is a valid resource rule. + policyRule.APIGroups = apiGroups + policyRule.Resources = resources + policyRule.ResourceNames = resourceNames + isValid = true + } + if len(nonResourceURLs) != 0 { + // This rule is a valid non-resource rule. + policyRule.NonResourceURLs = nonResourceURLs + isValid = true + } + if isValid { + return policyRule + } + return nil +} + +func intersectVerbs(verbs1, verbs2 []string) []string { + for _, verb := range verbs1 { + if verb == rbacv1.VerbAll { + return verbs2 + } + } + for _, verb := range verbs2 { + if verb == rbacv1.VerbAll { + return verbs1 + } + } + return intersection(verbs1, verbs2) +} + +func intersectAPIGroups(apiGroups1, apiGroups2 []string) []string { + for _, apiGroup := range apiGroups1 { + if apiGroup == rbacv1.APIGroupAll { + return apiGroups2 + } + } + for _, apiGroup := range apiGroups2 { + if apiGroup == rbacv1.APIGroupAll { + return apiGroups1 + } + } + return intersection(apiGroups1, apiGroups2) +} + +func intersectResources(resources1, resources2 []string) []string { + for _, resource := range resources1 { + if resource == rbacv1.ResourceAll { + return resources2 + } + } + for _, resource := range resources2 { + if resource == rbacv1.ResourceAll { + return resources1 + } + } + return intersection(resources1, resources2) +} + +func intersectResourceNames(resourceNames1, resourceNames2 []string) []string { + if len(resourceNames1) == 0 { + return resourceNames2 + } + if len(resourceNames2) == 0 { + return resourceNames1 + } + return intersection(resourceNames1, resourceNames2) +} + +func intersectNonResourceURLs(nonResourceURLs1, nonResourceURLs2 []string) []string { + for _, nonResourceURL := range nonResourceURLs1 { + if nonResourceURL == rbacv1.NonResourceAll { + return nonResourceURLs2 + } + } + for _, nonResourceURL := range nonResourceURLs2 { + if nonResourceURL == rbacv1.NonResourceAll { + return nonResourceURLs1 + } + } + return intersection(nonResourceURLs1, nonResourceURLs2) +} + +func intersection(s1, s2 []string) (inter []string) { + hash := make(map[string]struct{}) + for _, s := range s1 { + hash[s] = struct{}{} + } + for _, s := range s2 { + if _, ok := hash[s]; ok { + inter = append(inter, s) + } + } + return +} diff --git a/pkg/utils/intersection/intersection_test.go b/pkg/utils/intersection/intersection_test.go new file mode 100644 index 0000000..3e5c0b7 --- /dev/null +++ b/pkg/utils/intersection/intersection_test.go @@ -0,0 +1,328 @@ +/* +Copyright 2020 The Bulward Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package intersection + +import ( + "testing" + + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" +) + +func TestIntersectPolicyRule(t *testing.T) { + tests := []struct { + description string + rule1, rule2, expectedIntersectionRule *rbacv1.PolicyRule + }{ + { + description: "apiGroups don't match", + rule1: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + rule2: &rbacv1.PolicyRule{ + APIGroups: []string{"storage.bulward.io"}, + Resources: []string{"projects", "organizations"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + expectedIntersectionRule: nil, + }, + { + description: "resources don't match", + rule1: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + rule2: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"organizations"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + expectedIntersectionRule: nil, + }, + { + description: "verbs don't match", + rule1: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + Verbs: []string{"patch", "delete"}, + }, + rule2: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"organizations"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + expectedIntersectionRule: nil, + }, + { + description: "apiGroup subset", + rule1: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io", "storage.bulward.io"}, + Resources: []string{rbacv1.ResourceAll}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + rule2: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + expectedIntersectionRule: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + }, + { + description: "apiGroup subset", + rule1: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io", "storage.bulward.io"}, + Resources: []string{rbacv1.ResourceAll}, + ResourceNames: []string{}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + rule2: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + ResourceNames: []string{"aaa", "bbb"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + expectedIntersectionRule: &rbacv1.PolicyRule{ + APIGroups: []string{"apiserver.bulward.io"}, + Resources: []string{"projects"}, + ResourceNames: []string{"aaa", "bbb"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + }, + { + description: "non-resource rule", + rule1: &rbacv1.PolicyRule{ + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + NonResourceURLs: []string{rbacv1.NonResourceAll}, + }, + rule2: &rbacv1.PolicyRule{ + Verbs: []string{"get", "list", "watch", "create", "update"}, + NonResourceURLs: []string{"resource/aaa"}, + }, + expectedIntersectionRule: &rbacv1.PolicyRule{ + Verbs: []string{"get", "list", "watch", "create", "update"}, + NonResourceURLs: []string{"resource/aaa"}, + }, + }, + } + for _, test := range tests { + assert.Equal(t, test.expectedIntersectionRule, intersectPolicyRule(test.rule1, test.rule2)) + + } +} + +func TestIntersectVerbs(t *testing.T) { + tests := []struct { + verbs1, verbs2, expectedIntersection []string + }{ + { + verbs1: []string{"aaa"}, + verbs2: []string{rbacv1.VerbAll}, + expectedIntersection: []string{"aaa"}, + }, + { + verbs1: []string{}, + verbs2: []string{rbacv1.VerbAll}, + expectedIntersection: []string{}, + }, + { + verbs1: []string{"ccc"}, + verbs2: []string{"bbb", "aaa"}, + expectedIntersection: []string{}, + }, + { + verbs1: []string{"ccc", "aaa"}, + verbs2: []string{"bbb", "aaa"}, + expectedIntersection: []string{"aaa"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersectVerbs(test.verbs1, test.verbs2)) + } +} + +func TestIntersectAPIGroups(t *testing.T) { + tests := []struct { + apiGroups1, apiGroups2, expectedIntersection []string + }{ + { + apiGroups1: []string{"aaa.io"}, + apiGroups2: []string{rbacv1.APIGroupAll}, + expectedIntersection: []string{"aaa.io"}, + }, + { + apiGroups1: []string{}, + apiGroups2: []string{rbacv1.APIGroupAll}, + expectedIntersection: []string{}, + }, + { + apiGroups1: []string{"ccc.io"}, + apiGroups2: []string{"bbb.io", "aaa.io"}, + expectedIntersection: []string{}, + }, + { + apiGroups1: []string{"ccc.io", "aaa.io"}, + apiGroups2: []string{"bbb.io", "aaa.io"}, + expectedIntersection: []string{"aaa.io"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersectAPIGroups(test.apiGroups1, test.apiGroups2)) + } +} + +func TestIntersectResources(t *testing.T) { + tests := []struct { + resources1, resources2, expectedIntersection []string + }{ + { + resources1: []string{"aaa"}, + resources2: []string{rbacv1.ResourceAll}, + expectedIntersection: []string{"aaa"}, + }, + { + resources1: []string{}, + resources2: []string{rbacv1.ResourceAll}, + expectedIntersection: []string{}, + }, + { + resources1: []string{"ccc"}, + resources2: []string{"bbb", "aaa"}, + expectedIntersection: []string{}, + }, + { + resources1: []string{"ccc", "aaa"}, + resources2: []string{"bbb", "aaa"}, + expectedIntersection: []string{"aaa"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersectResources(test.resources1, test.resources2)) + } +} + +func TestIntersectResourceNames(t *testing.T) { + tests := []struct { + resourceNames1, resourceNames2, expectedIntersection []string + }{ + { + resourceNames1: []string{"aaa"}, + resourceNames2: []string{}, + expectedIntersection: []string{"aaa"}, + }, + { + resourceNames1: []string{}, + resourceNames2: []string{"bbb"}, + expectedIntersection: []string{"bbb"}, + }, + { + resourceNames1: []string{"ccc"}, + resourceNames2: []string{"bbb", "aaa"}, + expectedIntersection: []string{}, + }, + { + resourceNames1: []string{"ccc", "aaa"}, + resourceNames2: []string{"bbb", "aaa"}, + expectedIntersection: []string{"aaa"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersectResourceNames(test.resourceNames1, test.resourceNames2)) + } +} + +func TestIntersectNonResourceURLs(t *testing.T) { + tests := []struct { + nonResourceURLs1, nonResourceURLs2, expectedIntersection []string + }{ + { + nonResourceURLs1: []string{"aaa"}, + nonResourceURLs2: []string{rbacv1.NonResourceAll}, + expectedIntersection: []string{"aaa"}, + }, + { + nonResourceURLs1: []string{}, + nonResourceURLs2: []string{rbacv1.NonResourceAll}, + expectedIntersection: []string{}, + }, + { + nonResourceURLs1: []string{"ccc"}, + nonResourceURLs2: []string{"bbb", "aaa"}, + expectedIntersection: []string{}, + }, + { + nonResourceURLs1: []string{"ccc", "aaa"}, + nonResourceURLs2: []string{"bbb", "aaa"}, + expectedIntersection: []string{"aaa"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersectNonResourceURLs(test.nonResourceURLs1, test.nonResourceURLs2)) + } +} + +func TestIntersection(t *testing.T) { + tests := []struct { + s1, s2, expectedIntersection []string + }{ + { + s1: []string{"aaa"}, + s2: []string{}, + expectedIntersection: []string{}, + }, + { + s1: []string{}, + s2: []string{"bbb"}, + expectedIntersection: []string{}, + }, + { + s1: []string{"aaa"}, + s2: []string{"bbb"}, + expectedIntersection: []string{}, + }, + { + s1: []string{"aaa", "bbb"}, + s2: []string{"aaa"}, + expectedIntersection: []string{"aaa"}, + }, + { + s1: []string{"aaa", "bbb", "ccc"}, + s2: []string{"aaa", "ddd"}, + expectedIntersection: []string{"aaa"}, + }, + { + s1: []string{"aaa", "bbb", "ccc", "ddd"}, + s2: []string{"aaa", "ddd"}, + expectedIntersection: []string{"aaa", "ddd"}, + }, + } + + for _, test := range tests { + assert.ElementsMatch(t, test.expectedIntersection, intersection(test.s1, test.s2)) + } +} From 5c57d8f8730754067afa860609ee2729688c43bb Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Tue, 4 Aug 2020 18:39:32 +0800 Subject: [PATCH 2/6] Small fix --- .../controllers/organizationrole_controller.go | 10 ++++++---- pkg/manager/manager.go | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/manager/internal/controllers/organizationrole_controller.go b/pkg/manager/internal/controllers/organizationrole_controller.go index b63959c..72ee3e8 100644 --- a/pkg/manager/internal/controllers/organizationrole_controller.go +++ b/pkg/manager/internal/controllers/organizationrole_controller.go @@ -64,6 +64,12 @@ func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e return ctrl.Result{}, nil } +func (r *OrganizationRoleReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&corev1alpha1.OrganizationRole{}). + Complete(r) +} + func (r *OrganizationRoleReconciler) handleDeletion(ctx context.Context, log logr.Logger, organizationRole *corev1alpha1.OrganizationRole) error { // Update the OrganizationRole Status to Terminating. readyCondition, _ := organizationRole.Status.GetCondition(corev1alpha1.OrganizationRoleReady) @@ -82,7 +88,3 @@ func (r *OrganizationRoleReconciler) handleDeletion(ctx context.Context, log log } return nil } - -// -// func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole) error { -// } diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index d5aac31..9572cc8 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -108,6 +108,14 @@ func run(flags *flags, log logr.Logger) error { return fmt.Errorf("creating OrganizationRoleTemplate controller: %w", err) } + if err = (&controllers.OrganizationRoleReconciler{ + Client: mgr.GetClient(), + Log: log.WithName("controllers").WithName("OrganizationRole"), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + return fmt.Errorf("creating OrganizationRole controller: %w", err) + } + if err = (&controllers.ProjectReconciler{ Client: mgr.GetClient(), Log: log.WithName("controllers").WithName("Project"), From 3e3e0f36fd54b087f324d1f58b0954120abeaef8 Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Wed, 5 Aug 2020 18:31:35 +0800 Subject: [PATCH 3/6] Update --- config/manager/crd/kustomization.yaml | 1 + .../organizationrole_controller.go | 77 +++++++++++++++++++ .../organizationroletemplate_controller.go | 34 ++++---- 3 files changed, 97 insertions(+), 15 deletions(-) diff --git a/config/manager/crd/kustomization.yaml b/config/manager/crd/kustomization.yaml index 73abcb7..065c6c3 100644 --- a/config/manager/crd/kustomization.yaml +++ b/config/manager/crd/kustomization.yaml @@ -3,6 +3,7 @@ # It should be run by config/default resources: - bases/bulward.io_organizationroletemplates.yaml + - bases/bulward.io_organizationroles.yaml - bases/bulward.io_projectroletemplates.yaml - bases/storage.bulward.io_organizations.yaml - bases/storage.bulward.io_projects.yaml diff --git a/pkg/manager/internal/controllers/organizationrole_controller.go b/pkg/manager/internal/controllers/organizationrole_controller.go index 72ee3e8..f37a9e0 100644 --- a/pkg/manager/internal/controllers/organizationrole_controller.go +++ b/pkg/manager/internal/controllers/organizationrole_controller.go @@ -22,13 +22,19 @@ import ( "github.com/go-logr/logr" "k8c.io/utils/pkg/util" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" corev1alpha1 "k8c.io/bulward/pkg/apis/core/v1alpha1" + "k8c.io/bulward/pkg/utils/intersection" ) type OrganizationRoleReconciler struct { @@ -61,12 +67,50 @@ func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } } + if err := r.reconcileRole(ctx, organizationRole); err != nil { + return ctrl.Result{}, fmt.Errorf("reconciling Role: %w", err) + } + + if !organizationRole.IsReady() { + // Update OrganizationRole Status + organizationRole.Status.ObservedGeneration = organizationRole.Generation + organizationRole.Status.SetCondition(corev1alpha1.OrganizationRoleCondition{ + Type: corev1alpha1.OrganizationRoleReady, + Status: corev1alpha1.ConditionTrue, + Reason: "SetupComplete", + Message: "OrganizationRole setup is complete.", + }) + if err := r.Status().Update(ctx, organizationRole); err != nil { + return ctrl.Result{}, fmt.Errorf("updating OrganizationRole status: %w", err) + } + } + return ctrl.Result{}, nil } func (r *OrganizationRoleReconciler) SetupWithManager(mgr ctrl.Manager) error { + enqueueAllRoles := &handler.EnqueueRequestsFromMapFunc{ + ToRequests: handler.ToRequestsFunc(func(mapObject handler.MapObject) (out []ctrl.Request) { + organizationRoles := &corev1alpha1.OrganizationRoleList{} + if err := r.Client.List(context.Background(), organizationRoles); err != nil { + // This will makes the manager crashes, and it will restart and reconcile all objects again. + panic(fmt.Errorf("listting OrganizationRoles: %w", err)) + } + for _, organizationRole := range organizationRoles.Items { + out = append(out, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: organizationRole.Name, + Namespace: organizationRole.Namespace, + }, + }) + } + return + }), + } return ctrl.NewControllerManagedBy(mgr). For(&corev1alpha1.OrganizationRole{}). + Owns(&rbacv1.Role{}). + Watches(&source.Kind{Type: &corev1alpha1.OrganizationRoleTemplate{}}, enqueueAllRoles). Complete(r) } @@ -88,3 +132,36 @@ func (r *OrganizationRoleReconciler) handleDeletion(ctx context.Context, log log } return nil } + +func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole) error { + organizationRoleTemplates := &corev1alpha1.OrganizationRoleTemplateList{} + if err := r.List(ctx, organizationRoleTemplates); err != nil { + return err + } + var maxRules []rbacv1.PolicyRule + for _, template := range organizationRoleTemplates.Items { + if template.IsReady() { + maxRules = append(maxRules, template.Spec.Rules...) + } + } + policyRoles := intersection.IntersectPolicyRules(maxRules, organizationRole.Spec.Rules) + + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: organizationRole.Name, + Namespace: organizationRole.Namespace, + }, + Rules: policyRoles, + } + desiredRole := role.DeepCopy() + if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, role, func() error { + if err := controllerutil.SetControllerReference(organizationRole, role, r.Scheme); err != nil { + return fmt.Errorf("setting owner reference: %w", err) + } + role.Rules = desiredRole.Rules + return nil + }); err != nil { + return err + } + return nil +} diff --git a/pkg/manager/internal/controllers/organizationroletemplate_controller.go b/pkg/manager/internal/controllers/organizationroletemplate_controller.go index f307a75..1989bb5 100644 --- a/pkg/manager/internal/controllers/organizationroletemplate_controller.go +++ b/pkg/manager/internal/controllers/organizationroletemplate_controller.go @@ -195,14 +195,16 @@ func (r *OrganizationRoleTemplateReconciler) handleDeletion(ctx context.Context, func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization) error { // Reconcile Role. - role := &rbacv1.Role{ + organizationRole := &corev1alpha1.OrganizationRole{ ObjectMeta: metav1.ObjectMeta{ Name: organizationRoleTemplate.Name, Namespace: organization.Status.Namespace.Name, }, - Rules: organizationRoleTemplate.Spec.Rules, + Spec: corev1alpha1.OrganizationRoleSpec{ + Rules: organizationRoleTemplate.Spec.Rules, + }, } - if err := r.reconcileRole(ctx, role, organizationRoleTemplate); err != nil { + if err := r.reconcileRole(ctx, organizationRole, organizationRoleTemplate); err != nil { return err } @@ -217,7 +219,7 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx co RoleRef: rbacv1.RoleRef{ APIGroup: rbacv1.GroupName, Kind: "Role", - Name: role.Name, + Name: organizationRole.Name, }, } if err := r.reconcileRoleBinding(ctx, roleBinding, organizationRoleTemplate); err != nil { @@ -228,15 +230,17 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx co } func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization, project *storagev1alpha1.Project) error { - // Reconcile Role. - role := &rbacv1.Role{ + // Reconcile OrganizationRole. + organizationRole := &corev1alpha1.OrganizationRole{ ObjectMeta: metav1.ObjectMeta{ Name: organizationRoleTemplate.Name, Namespace: project.Status.Namespace.Name, }, - Rules: organizationRoleTemplate.Spec.Rules, + Spec: corev1alpha1.OrganizationRoleSpec{ + Rules: organizationRoleTemplate.Spec.Rules, + }, } - if err := r.reconcileRole(ctx, role, organizationRoleTemplate); err != nil { + if err := r.reconcileRole(ctx, organizationRole, organizationRoleTemplate); err != nil { return err } @@ -252,7 +256,7 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context RoleRef: rbacv1.RoleRef{ APIGroup: rbacv1.GroupName, Kind: "Role", - Name: role.Name, + Name: organizationRole.Name, }, } if err := r.reconcileRoleBinding(ctx, roleBinding, organizationRoleTemplate); err != nil { @@ -262,17 +266,17 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context return nil } -func (r *OrganizationRoleTemplateReconciler) reconcileRole(ctx context.Context, role *rbacv1.Role, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate) error { - desiredRole := role.DeepCopy() - if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, role, func() error { +func (r *OrganizationRoleTemplateReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate) error { + desiredRole := organizationRole.DeepCopy() + if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, organizationRole, func() error { if err := controllerutil.SetControllerReference( - organizationRoleTemplate, role, r.Scheme); err != nil { + organizationRoleTemplate, organizationRole, r.Scheme); err != nil { return fmt.Errorf("set controller reference for Role: %w", err) } - role.Rules = desiredRole.Rules + organizationRole.Spec.Rules = desiredRole.Spec.Rules return nil }); err != nil { - return fmt.Errorf("creating or updating Role: %w", err) + return fmt.Errorf("creating or updating OrganizationRole: %w", err) } return nil } From 1b8f6c28ebc85ea122ae710ab10c22cef05a9254 Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Thu, 6 Aug 2020 15:44:13 +0800 Subject: [PATCH 4/6] Update --- config/manager/rbac/role.yaml | 19 ++ config/samples/organizationrole.yaml | 19 ++ .../organizationrole_controller.go | 33 ++-- .../organizationroletemplate_controller.go | 62 +++++-- pkg/templates/organizationrole.go | 7 + test/organizationrole_test.go | 164 ++++++++++++++++++ 6 files changed, 278 insertions(+), 26 deletions(-) create mode 100644 config/samples/organizationrole.yaml create mode 100644 test/organizationrole_test.go diff --git a/config/manager/rbac/role.yaml b/config/manager/rbac/role.yaml index 9eefce0..c4c1f14 100644 --- a/config/manager/rbac/role.yaml +++ b/config/manager/rbac/role.yaml @@ -30,6 +30,25 @@ rules: - patch - update - watch +- apiGroups: + - bulward.io + resources: + - organizationroles + verbs: + - bind + - create + - get + - list + - update + - watch +- apiGroups: + - bulward.io + resources: + - organizationroles/status + verbs: + - get + - patch + - update - apiGroups: - bulward.io resources: diff --git a/config/samples/organizationrole.yaml b/config/samples/organizationrole.yaml new file mode 100644 index 0000000..d97322c --- /dev/null +++ b/config/samples/organizationrole.yaml @@ -0,0 +1,19 @@ +apiVersion: bulward.io/v1alpha1 +kind: OrganizationRole +metadata: + name: rbac-sub-role + namespace: organization-a +spec: + rules: + - apiGroups: + - bulward.io + resources: + - organizationroles + - organizationfjl + verbs: + - get + - create + - update + - patch + - bind + - delete diff --git a/pkg/manager/internal/controllers/organizationrole_controller.go b/pkg/manager/internal/controllers/organizationrole_controller.go index f37a9e0..794ca6f 100644 --- a/pkg/manager/internal/controllers/organizationrole_controller.go +++ b/pkg/manager/internal/controllers/organizationrole_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "reflect" "github.com/go-logr/logr" "k8c.io/utils/pkg/util" @@ -43,6 +44,9 @@ type OrganizationRoleReconciler struct { Scheme *runtime.Scheme } +// +kubebuilder:rbac:groups=bulward.io,resources=organizationroles,verbs=get;list;watch;update +// +kubebuilder:rbac:groups=bulward.io,resources=organizationroles/status,verbs=get;update;patch + func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { ctx := context.Background() log := r.Log.WithValues("organizationRole", req.NamespacedName) @@ -67,10 +71,16 @@ func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } } - if err := r.reconcileRole(ctx, organizationRole); err != nil { + acceptedRules, err := r.reconcileRole(ctx, organizationRole) + if err != nil { return ctrl.Result{}, fmt.Errorf("reconciling Role: %w", err) } + var isChanged bool + if !reflect.DeepEqual(organizationRole.Status.AcceptedRules, acceptedRules) { + organizationRole.Status.AcceptedRules = acceptedRules + isChanged = true + } if !organizationRole.IsReady() { // Update OrganizationRole Status organizationRole.Status.ObservedGeneration = organizationRole.Generation @@ -80,6 +90,9 @@ func (r *OrganizationRoleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e Reason: "SetupComplete", Message: "OrganizationRole setup is complete.", }) + isChanged = true + } + if isChanged { if err := r.Status().Update(ctx, organizationRole); err != nil { return ctrl.Result{}, fmt.Errorf("updating OrganizationRole status: %w", err) } @@ -133,16 +146,14 @@ func (r *OrganizationRoleReconciler) handleDeletion(ctx context.Context, log log return nil } -func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole) error { +func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole) (acceptedRules []rbacv1.PolicyRule, err error) { organizationRoleTemplates := &corev1alpha1.OrganizationRoleTemplateList{} - if err := r.List(ctx, organizationRoleTemplates); err != nil { - return err + if err = r.List(ctx, organizationRoleTemplates); err != nil { + return } var maxRules []rbacv1.PolicyRule for _, template := range organizationRoleTemplates.Items { - if template.IsReady() { - maxRules = append(maxRules, template.Spec.Rules...) - } + maxRules = append(maxRules, template.Spec.Rules...) } policyRoles := intersection.IntersectPolicyRules(maxRules, organizationRole.Spec.Rules) @@ -154,14 +165,14 @@ func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organiza Rules: policyRoles, } desiredRole := role.DeepCopy() - if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, role, func() error { - if err := controllerutil.SetControllerReference(organizationRole, role, r.Scheme); err != nil { + if _, err = controllerutil.CreateOrUpdate(ctx, r.Client, role, func() error { + if err = controllerutil.SetControllerReference(organizationRole, role, r.Scheme); err != nil { return fmt.Errorf("setting owner reference: %w", err) } role.Rules = desiredRole.Rules return nil }); err != nil { - return err + return nil, err } - return nil + return role.Rules, nil } diff --git a/pkg/manager/internal/controllers/organizationroletemplate_controller.go b/pkg/manager/internal/controllers/organizationroletemplate_controller.go index 1989bb5..f6c434a 100644 --- a/pkg/manager/internal/controllers/organizationroletemplate_controller.go +++ b/pkg/manager/internal/controllers/organizationroletemplate_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "reflect" + "strings" "github.com/go-logr/logr" rbacv1 "k8s.io/api/rbac/v1" @@ -49,6 +50,7 @@ type OrganizationRoleTemplateReconciler struct { // +kubebuilder:rbac:groups=storage.bulward.io,resources=projects,verbs=get;list;watch;update // The following permission of apiserver.bulward.io is needed for service account to create role for owner to access projects. // +kubebuilder:rbac:groups=apiserver.bulward.io,resources=projects,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=bulward.io,resources=organizationroles,verbs=get;list;watch;create;update;bind // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=get;list;watch;create;update;patch;delete;bind // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete @@ -74,15 +76,23 @@ func (r *OrganizationRoleTemplateReconciler) Reconcile(req ctrl.Request) (ctrl.R } // Reconcile Role/RoleBindings in Organization namespaces. + var unSetUpOrganization []string if organizationRoleTemplate.HasScope(corev1alpha1.RoleTemplateScopeOrganization) { for _, organization := range organizations.Items { if !organization.IsReady() { // skip Unready Organizations. continue } - if err := r.reconcileRBACForOrganization(ctx, organizationRoleTemplate, &organization); err != nil { + organizationRole, err := r.reconcileRBACForOrganization(ctx, organizationRoleTemplate, &organization) + if err != nil { return ctrl.Result{}, fmt.Errorf("reconcling Organization RBAC: %w", err) } + + if !organizationRole.IsReady() { + unSetUpOrganization = append(unSetUpOrganization, organization.Name) + continue + } + targets = append(targets, corev1alpha1.RoleTemplateTarget{ Kind: organization.Kind, APIGroup: organization.GroupVersionKind().Group, @@ -108,9 +118,14 @@ func (r *OrganizationRoleTemplateReconciler) Reconcile(req ctrl.Request) (ctrl.R if !project.IsReady() { continue } - if err := r.reconcileRBACForProject(ctx, organizationRoleTemplate, &organization, &project); err != nil { + organizationRole, err := r.reconcileRBACForProject(ctx, organizationRoleTemplate, &organization, &project) + if err != nil { return ctrl.Result{}, fmt.Errorf("reconcling Project RBAC: %w", err) } + if !organizationRole.IsReady() { + unSetUpOrganization = append(unSetUpOrganization, organization.Name) + continue + } targets = append(targets, corev1alpha1.RoleTemplateTarget{ Kind: project.Kind, APIGroup: project.GroupVersionKind().Group, @@ -126,8 +141,16 @@ func (r *OrganizationRoleTemplateReconciler) Reconcile(req ctrl.Request) (ctrl.R organizationRoleTemplate.Status.Targets = targets changed = true } - if !organizationRoleTemplate.IsReady() { - // Update OrganizationRoleTemplate Status + if len(unSetUpOrganization) != 0 { + organizationRoleTemplate.Status.ObservedGeneration = organizationRoleTemplate.Generation + organizationRoleTemplate.Status.SetCondition(corev1alpha1.OrganizationRoleTemplateCondition{ + Type: corev1alpha1.OrganizationRoleTemplateReady, + Status: corev1alpha1.ConditionFalse, + Reason: "SetupNotComplete", + Message: fmt.Sprintf("RBAC setup is not complete for some organizations [%s]", strings.Join(unSetUpOrganization, ",")), + }) + changed = true + } else if !organizationRoleTemplate.IsReady() { organizationRoleTemplate.Status.ObservedGeneration = organizationRoleTemplate.Generation organizationRoleTemplate.Status.SetCondition(corev1alpha1.OrganizationRoleTemplateCondition{ Type: corev1alpha1.OrganizationRoleTemplateReady, @@ -167,6 +190,7 @@ func (r *OrganizationRoleTemplateReconciler) SetupWithManager(mgr ctrl.Manager) return ctrl.NewControllerManagedBy(mgr). For(&corev1alpha1.OrganizationRoleTemplate{}). + Owns(&corev1alpha1.OrganizationRole{}). Watches(&source.Kind{Type: &storagev1alpha1.Organization{}}, enqueueAllTemplates). Watches(&source.Kind{Type: &storagev1alpha1.Project{}}, enqueueAllTemplates). Complete(r) @@ -193,7 +217,7 @@ func (r *OrganizationRoleTemplateReconciler) handleDeletion(ctx context.Context, return nil } -func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization) error { +func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization) (*corev1alpha1.OrganizationRole, error) { // Reconcile Role. organizationRole := &corev1alpha1.OrganizationRole{ ObjectMeta: metav1.ObjectMeta{ @@ -204,8 +228,12 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx co Rules: organizationRoleTemplate.Spec.Rules, }, } - if err := r.reconcileRole(ctx, organizationRole, organizationRoleTemplate); err != nil { - return err + if err := r.reconcileOrganizationRole(ctx, organizationRole, organizationRoleTemplate); err != nil { + return nil, err + } + + if !organizationRole.IsReady() { + return organizationRole, nil } // Reconcile RoleBindings. @@ -223,13 +251,13 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForOrganization(ctx co }, } if err := r.reconcileRoleBinding(ctx, roleBinding, organizationRoleTemplate); err != nil { - return err + return organizationRole, err } } - return nil + return organizationRole, nil } -func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization, project *storagev1alpha1.Project) error { +func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context.Context, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate, organization *storagev1alpha1.Organization, project *storagev1alpha1.Project) (*corev1alpha1.OrganizationRole, error) { // Reconcile OrganizationRole. organizationRole := &corev1alpha1.OrganizationRole{ ObjectMeta: metav1.ObjectMeta{ @@ -240,8 +268,12 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context Rules: organizationRoleTemplate.Spec.Rules, }, } - if err := r.reconcileRole(ctx, organizationRole, organizationRoleTemplate); err != nil { - return err + if err := r.reconcileOrganizationRole(ctx, organizationRole, organizationRoleTemplate); err != nil { + return nil, err + } + + if !organizationRole.IsReady() { + return organizationRole, nil } // Reconcile RoleBindings. @@ -260,13 +292,13 @@ func (r *OrganizationRoleTemplateReconciler) reconcileRBACForProject(ctx context }, } if err := r.reconcileRoleBinding(ctx, roleBinding, organizationRoleTemplate); err != nil { - return err + return organizationRole, err } } - return nil + return organizationRole, nil } -func (r *OrganizationRoleTemplateReconciler) reconcileRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate) error { +func (r *OrganizationRoleTemplateReconciler) reconcileOrganizationRole(ctx context.Context, organizationRole *corev1alpha1.OrganizationRole, organizationRoleTemplate *corev1alpha1.OrganizationRoleTemplate) error { desiredRole := organizationRole.DeepCopy() if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, organizationRole, func() error { if err := controllerutil.SetControllerReference( diff --git a/pkg/templates/organizationrole.go b/pkg/templates/organizationrole.go index 5c2d161..22ebed4 100644 --- a/pkg/templates/organizationrole.go +++ b/pkg/templates/organizationrole.go @@ -74,6 +74,13 @@ func RBACAdminOrganizationRoleTemplate() *corev1alpha1.OrganizationRoleTemplate corev1alpha1.BindToOwners, }, Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"bulward.io"}, + Resources: []string{"organizationroles"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete", "bind"}, + }, + // This is still needed for owner to get the role in storage_project_test.go, this permission will be revoked + // once we have both OrganizationRole and ProjectRole implemented. { APIGroups: []string{rbacv1.GroupName}, Resources: []string{"roles"}, diff --git a/test/organizationrole_test.go b/test/organizationrole_test.go new file mode 100644 index 0000000..ae8c5dd --- /dev/null +++ b/test/organizationrole_test.go @@ -0,0 +1,164 @@ +/* +Copyright 2020 The Bulward Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "context" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8c.io/utils/pkg/testutil" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + controllerruntime "sigs.k8s.io/controller-runtime" + + corev1alpha1 "k8c.io/bulward/pkg/apis/core/v1alpha1" + storagev1alpha1 "k8c.io/bulward/pkg/apis/storage/v1alpha1" +) + +func TestOrganizationRole(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + cfg, err := controllerruntime.GetConfig() + require.NoError(t, err) + cl := testutil.NewRecordingClient(t, cfg, testScheme, testutil.CleanUpStrategy(cleanUpStrategy)) + t.Cleanup(cl.CleanUpFunc(ctx)) + testName := strings.ToLower(t.Name()) + + organizationOwner := rbacv1.Subject{ + Kind: rbacv1.UserKind, + APIGroup: rbacv1.GroupName, + Name: "Organization Owner", + } + + org := &storagev1alpha1.Organization{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + }, + Spec: storagev1alpha1.OrganizationSpec{ + Metadata: &storagev1alpha1.OrganizationMetadata{ + DisplayName: "berlin", + Description: "a humble organization of German capital", + }, + Owners: []rbacv1.Subject{organizationOwner}, + }, + } + require.NoError(t, cl.Create(ctx, org)) + require.NoError(t, testutil.WaitUntilReady(ctx, cl, org)) + + t.Log("Create an OrganizationRoleTemplate object to test") + organizationRoleTemplate := &corev1alpha1.OrganizationRoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + }, + Spec: corev1alpha1.OrganizationRoleTemplateSpec{ + Scopes: []corev1alpha1.RoleTemplateScope{ + corev1alpha1.RoleTemplateScopeOrganization, + corev1alpha1.RoleTemplateScopeProject, + }, + BindTo: []corev1alpha1.BindingType{ + corev1alpha1.BindToOwners, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"example.app.io"}, + Resources: []string{"resources", "apps"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + }, + }, + } + require.NoError(t, cl.Create(ctx, organizationRoleTemplate)) + require.NoError(t, testutil.WaitUntilReady(ctx, cl, organizationRoleTemplate)) + + t.Log("Check OrganizationRoles and Roles are ready/created.") + organizationRole := &corev1alpha1.OrganizationRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: org.Status.Namespace.Name, + }, + } + require.NoError(t, testutil.WaitUntilReady(ctx, cl, organizationRole)) + testutil.LogObject(t, organizationRole) + assert.ElementsMatch(t, organizationRoleTemplate.Spec.Rules, organizationRole.Status.AcceptedRules) + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: org.Status.Namespace.Name, + }, + } + require.NoError(t, testutil.WaitUntilFound(ctx, cl, role)) + assert.ElementsMatch(t, organizationRoleTemplate.Spec.Rules, role.Rules) + + t.Log("Owner create OrganizationRole object to enable sub-role") + cfg.Impersonate = rest.ImpersonationConfig{ + UserName: organizationOwner.Name, + } + ownerClient := testutil.NewRecordingClient(t, cfg, testScheme, testutil.CleanUpStrategy(cleanUpStrategy)) + viewRole := &corev1alpha1.OrganizationRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-view-role", testName), + Namespace: org.Status.Namespace.Name, + }, + Spec: corev1alpha1.OrganizationRoleSpec{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"example.app.io"}, + Resources: []string{"resources", "apps"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + } + require.NoError(t, ownerClient.Create(ctx, viewRole)) + require.NoError(t, testutil.WaitUntilReady(ctx, cl, viewRole)) + role = &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-view-role", testName), + Namespace: org.Status.Namespace.Name, + }, + } + require.NoError(t, testutil.WaitUntilFound(ctx, cl, role)) + assert.ElementsMatch(t, viewRole.Spec.Rules, role.Rules) + assert.ElementsMatch(t, viewRole.Status.AcceptedRules, role.Rules) + + t.Log("Organization revoke partial permissions") + organizationRoleTemplate.Spec.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{"example.app.io"}, + Resources: []string{"apps"}, + Verbs: []string{"get"}, + }, + } + require.NoError(t, cl.Update(ctx, organizationRoleTemplate)) + require.NoError(t, cl.WaitUntil(ctx, role, func() (done bool, err error) { + return reflect.DeepEqual(role.Rules, organizationRoleTemplate.Spec.Rules), nil + }), "subRole is not revoked.") + + t.Log("Clean up") + require.NoError(t, testutil.DeleteAndWaitUntilNotFound(ctx, cl, organizationRoleTemplate)) + require.NoError(t, cl.WaitUntil(ctx, viewRole, func() (done bool, err error) { + return len(viewRole.Status.AcceptedRules) == 0, nil + }), "subRole is not revoked.") + require.NoError(t, cl.Delete(ctx, viewRole)) + require.NoError(t, cl.WaitUntilNotFound(ctx, role)) +} From 4c792be6a8dcfd01476147ed91f16ea79c628b3d Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Thu, 13 Aug 2020 18:35:19 +0800 Subject: [PATCH 5/6] Implement feedback --- .../organizationrole_controller.go | 4 ++-- .../intersect.go} | 20 ++++++------------- .../intersect_test.go} | 11 +++++----- 3 files changed, 13 insertions(+), 22 deletions(-) rename pkg/utils/{intersection/intersection.go => intersect/intersect.go} (89%) rename pkg/utils/{intersection/intersection_test.go => intersect/intersect_test.go} (96%) diff --git a/pkg/manager/internal/controllers/organizationrole_controller.go b/pkg/manager/internal/controllers/organizationrole_controller.go index 794ca6f..21a4f0c 100644 --- a/pkg/manager/internal/controllers/organizationrole_controller.go +++ b/pkg/manager/internal/controllers/organizationrole_controller.go @@ -35,7 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" corev1alpha1 "k8c.io/bulward/pkg/apis/core/v1alpha1" - "k8c.io/bulward/pkg/utils/intersection" + "k8c.io/bulward/pkg/utils/intersect" ) type OrganizationRoleReconciler struct { @@ -155,7 +155,7 @@ func (r *OrganizationRoleReconciler) reconcileRole(ctx context.Context, organiza for _, template := range organizationRoleTemplates.Items { maxRules = append(maxRules, template.Spec.Rules...) } - policyRoles := intersection.IntersectPolicyRules(maxRules, organizationRole.Spec.Rules) + policyRoles := intersect.PolicyRules(maxRules, organizationRole.Spec.Rules) role := &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/utils/intersection/intersection.go b/pkg/utils/intersect/intersect.go similarity index 89% rename from pkg/utils/intersection/intersection.go rename to pkg/utils/intersect/intersect.go index 4d0d939..622eb05 100644 --- a/pkg/utils/intersection/intersection.go +++ b/pkg/utils/intersect/intersect.go @@ -14,17 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package intersection +package intersect import ( rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/sets" ) -func IntersectPolicyRules(rules1, rules2 []rbacv1.PolicyRule) []rbacv1.PolicyRule { +func PolicyRules(rules1, rules2 []rbacv1.PolicyRule) []rbacv1.PolicyRule { var newRules []rbacv1.PolicyRule for _, r1 := range rules1 { for _, r2 := range rules2 { - rule := intersectPolicyRule(&r1, &r2) + rule := PolicyRule(&r1, &r2) if rule != nil { newRules = append(newRules, *rule) } @@ -33,7 +34,7 @@ func IntersectPolicyRules(rules1, rules2 []rbacv1.PolicyRule) []rbacv1.PolicyRul return newRules } -func intersectPolicyRule(rule1, rule2 *rbacv1.PolicyRule) *rbacv1.PolicyRule { +func PolicyRule(rule1, rule2 *rbacv1.PolicyRule) *rbacv1.PolicyRule { verbs := intersectVerbs(rule1.Verbs, rule2.Verbs) if len(verbs) == 0 { return nil @@ -131,14 +132,5 @@ func intersectNonResourceURLs(nonResourceURLs1, nonResourceURLs2 []string) []str } func intersection(s1, s2 []string) (inter []string) { - hash := make(map[string]struct{}) - for _, s := range s1 { - hash[s] = struct{}{} - } - for _, s := range s2 { - if _, ok := hash[s]; ok { - inter = append(inter, s) - } - } - return + return sets.NewString(s1...).Intersection(sets.NewString(s2...)).List() } diff --git a/pkg/utils/intersection/intersection_test.go b/pkg/utils/intersect/intersect_test.go similarity index 96% rename from pkg/utils/intersection/intersection_test.go rename to pkg/utils/intersect/intersect_test.go index 3e5c0b7..6df55dd 100644 --- a/pkg/utils/intersection/intersection_test.go +++ b/pkg/utils/intersect/intersect_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package intersection +package intersect import ( "testing" @@ -85,7 +85,7 @@ func TestIntersectPolicyRule(t *testing.T) { expectedIntersectionRule: &rbacv1.PolicyRule{ APIGroups: []string{"apiserver.bulward.io"}, Resources: []string{"projects"}, - Verbs: []string{"get", "list", "watch", "create", "update"}, + Verbs: []string{"create", "get", "list", "update", "watch"}, }, }, { @@ -106,7 +106,7 @@ func TestIntersectPolicyRule(t *testing.T) { APIGroups: []string{"apiserver.bulward.io"}, Resources: []string{"projects"}, ResourceNames: []string{"aaa", "bbb"}, - Verbs: []string{"get", "list", "watch", "create", "update"}, + Verbs: []string{"create", "get", "list", "update", "watch"}, }, }, { @@ -120,14 +120,13 @@ func TestIntersectPolicyRule(t *testing.T) { NonResourceURLs: []string{"resource/aaa"}, }, expectedIntersectionRule: &rbacv1.PolicyRule{ - Verbs: []string{"get", "list", "watch", "create", "update"}, + Verbs: []string{"create", "get", "list", "update", "watch"}, NonResourceURLs: []string{"resource/aaa"}, }, }, } for _, test := range tests { - assert.Equal(t, test.expectedIntersectionRule, intersectPolicyRule(test.rule1, test.rule2)) - + assert.Equal(t, test.expectedIntersectionRule, PolicyRule(test.rule1, test.rule2)) } } From 56174c5e7b3ef9722e0b797d22150fb539db62a2 Mon Sep 17 00:00:00 2001 From: Jiacheng Xu Date: Thu, 13 Aug 2020 18:55:35 +0800 Subject: [PATCH 6/6] Implement the feedback --- test/organizationrole_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/organizationrole_test.go b/test/organizationrole_test.go index ae8c5dd..d45ec0d 100644 --- a/test/organizationrole_test.go +++ b/test/organizationrole_test.go @@ -81,8 +81,8 @@ func TestOrganizationRole(t *testing.T) { Rules: []rbacv1.PolicyRule{ { APIGroups: []string{"example.app.io"}, - Resources: []string{"resources", "apps"}, - Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + Resources: []string{"apps", "resources"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, }, }, }, @@ -123,7 +123,7 @@ func TestOrganizationRole(t *testing.T) { Rules: []rbacv1.PolicyRule{ { APIGroups: []string{"example.app.io"}, - Resources: []string{"resources", "apps"}, + Resources: []string{"apps", "resources"}, Verbs: []string{"get", "list", "watch"}, }, },