Skip to content

Commit

Permalink
Allow users to configure permissions for pipeline (#206)
Browse files Browse the repository at this point in the history
* feat: (197) Introduce pipeline permissions

* feat: (197) implement cleanup for user provided permission role

- if user removed pipeline.spec.rbac.permission, role and role binding
will be cleaned up

Co-authored-by: Rich Barton-Cooper <rich@syntasso.io>

---------

Co-authored-by: Rich Barton-Cooper <rich@syntasso.io>
  • Loading branch information
ChunyiLyu and richcooper95 authored Jul 25, 2024
1 parent 843f54c commit 83530b2
Show file tree
Hide file tree
Showing 14 changed files with 1,131 additions and 721 deletions.
321 changes: 199 additions & 122 deletions api/v1alpha1/pipeline_types.go

Large diffs are not rendered by default.

1,165 changes: 615 additions & 550 deletions api/v1alpha1/pipeline_types_test.go

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions api/v1alpha1/promise_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func (p *Promise) GetWorkloadGroupScheduling() []WorkloadGroupScheduling {
return workloadGroupScheduling
}

func (p *Promise) generatePipelinesObjects(workflowType Type, workflowAction Action, crd *apiextensionsv1.CustomResourceDefinition, resourceRequest *unstructured.Unstructured, logger logr.Logger) ([]PipelineJobResources, error) {
func (p *Promise) generatePipelinesObjects(workflowType Type, workflowAction Action, resourceRequest *unstructured.Unstructured, logger logr.Logger) ([]PipelineJobResources, error) {
promisePipelines, err := NewPipelinesMap(p, logger)
if err != nil {
return nil, err
Expand Down Expand Up @@ -311,11 +311,11 @@ func (p *Promise) generatePipelinesObjects(workflowType Type, workflowAction Act
}

func (p *Promise) GeneratePromisePipelines(workflowAction Action, logger logr.Logger) ([]PipelineJobResources, error) {
return p.generatePipelinesObjects(WorkflowTypePromise, workflowAction, nil, nil, logger)
return p.generatePipelinesObjects(WorkflowTypePromise, workflowAction, nil, logger)
}

func (p *Promise) GenerateResourcePipelines(workflowAction Action, crd *apiextensionsv1.CustomResourceDefinition, resourceRequest *unstructured.Unstructured, logger logr.Logger) ([]PipelineJobResources, error) {
return p.generatePipelinesObjects(WorkflowTypeResource, workflowAction, crd, resourceRequest, logger)
func (p *Promise) GenerateResourcePipelines(workflowAction Action, resourceRequest *unstructured.Unstructured, logger logr.Logger) ([]PipelineJobResources, error) {
return p.generatePipelinesObjects(WorkflowTypeResource, workflowAction, resourceRequest, logger)
}

func (p *Promise) HasPipeline(workflowType Type, workflowAction Action) bool {
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/promise_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ func (p *Promise) validatePipelines() error {
factory = pipeline.ForPromise(p, workflowAction)
}

if len(factory.ID) > 63 {
if len(factory.ID) > 60 {
return fmt.Errorf("%s.%s pipeline with name %q is too long. The name is used when generating resources "+
"for the pipeline,including the ServiceAccount which follows the format of \"%s-%s-%s-%s\", which cannot be longer than 63 characters in total",
"for the pipeline,including the ServiceAccount which follows the format of \"%s-%s-%s-%s\", which cannot be longer than 60 characters in total",
workflowType, workflowAction, pipeline.GetName(), p.GetName(), workflowType, workflowAction, pipeline.GetName())
}
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/promise_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ var _ = Describe("PromiseWebhook", func() {

BeforeEach(func() {
promise = newPromise()
maxLimit = 63 - len(promise.Name+"-resource-configure-")
maxLimit = 60 - len(promise.Name+"-resource-configure-")
})

It("returns an error when too long", func() {
Expand All @@ -177,7 +177,7 @@ var _ = Describe("PromiseWebhook", func() {
_, err = promise.ValidateCreate()
Expect(err).To(MatchError("resource.configure pipeline with name \"" + pipeline.GetName() + "\" is too long. " +
"The name is used when generating resources for the pipeline,including the ServiceAccount which follows the format of " +
"\"mypromise-resource-configure-" + pipeline.GetName() + "\", which cannot be longer than 63 characters in total"))
"\"mypromise-resource-configure-" + pipeline.GetName() + "\", which cannot be longer than 60 characters in total"))
})

It("succeeds when within the character limit", func() {
Expand Down
79 changes: 78 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions controllers/dynamic_resource_request_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (r *DynamicResourceRequestController) Reconcile(ctx context.Context, req ct
return addFinalizers(opts, rr, []string{workFinalizer, removeAllWorkflowJobsFinalizer, runDeleteWorkflowsFinalizer})
}

pipelineResources, err := promise.GenerateResourcePipelines(v1alpha1.WorkflowActionConfigure, r.CRD, rr, logger)
pipelineResources, err := promise.GenerateResourcePipelines(v1alpha1.WorkflowActionConfigure, rr, logger)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -167,7 +167,7 @@ func (r *DynamicResourceRequestController) deleteResources(o opts, promise *v1al
}

if controllerutil.ContainsFinalizer(resourceRequest, runDeleteWorkflowsFinalizer) {
pipelineResources, err := promise.GenerateResourcePipelines(v1alpha1.WorkflowActionDelete, r.CRD, resourceRequest, o.logger)
pipelineResources, err := promise.GenerateResourcePipelines(v1alpha1.WorkflowActionDelete, resourceRequest, o.logger)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
14 changes: 7 additions & 7 deletions controllers/dynamic_resource_request_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,16 @@ var _ = Describe("DynamicResourceRequestController", func() {
"kratix.io/promise-name": promise.GetName(),
}

resources := reconcileConfigureOptsArg.Resources[0].RequiredResources
resources := reconcileConfigureOptsArg.Resources[0].GetObjects()
By("creating a service account for pipeline", func() {
Expect(resources[0]).To(BeAssignableToTypeOf(&v1.ServiceAccount{}))
sa := resources[0].(*v1.ServiceAccount)
Expect(sa.GetLabels()).To(Equal(resourceLabels))
})

By("creates a role for the pipeline service account", func() {
Expect(resources[1]).To(BeAssignableToTypeOf(&rbacv1.Role{}))
role := resources[1].(*rbacv1.Role)
Expect(resources[2]).To(BeAssignableToTypeOf(&rbacv1.Role{}))
role := resources[2].(*rbacv1.Role)
Expect(role.GetLabels()).To(Equal(resourceLabels))
Expect(role.Rules).To(ConsistOf(
rbacv1.PolicyRule{
Expand All @@ -127,8 +127,8 @@ var _ = Describe("DynamicResourceRequestController", func() {
})

By("associates the new role with the new service account", func() {
Expect(resources[2]).To(BeAssignableToTypeOf(&rbacv1.RoleBinding{}))
binding := resources[2].(*rbacv1.RoleBinding)
Expect(resources[3]).To(BeAssignableToTypeOf(&rbacv1.RoleBinding{}))
binding := resources[3].(*rbacv1.RoleBinding)
Expect(binding.RoleRef.Name).To(Equal("redis-resource-configure-first-pipeline"))
Expect(binding.Subjects).To(HaveLen(1))
Expect(binding.Subjects[0]).To(Equal(rbacv1.Subject{
Expand All @@ -140,8 +140,8 @@ var _ = Describe("DynamicResourceRequestController", func() {
})

By("creates a config map with the promise scheduling in it", func() {
Expect(resources[3]).To(BeAssignableToTypeOf(&v1.ConfigMap{}))
configMap := resources[3].(*v1.ConfigMap)
Expect(resources[1]).To(BeAssignableToTypeOf(&v1.ConfigMap{}))
configMap := resources[1].(*v1.ConfigMap)
Expect(configMap.GetName()).To(Equal("destination-selectors-" + promise.GetName()))
Expect(configMap.GetNamespace()).To(Equal("default"))
Expect(configMap.GetLabels()).To(Equal(resourceLabels))
Expand Down
14 changes: 7 additions & 7 deletions controllers/promise_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,16 +404,16 @@ var _ = Describe("PromiseController", func() {
Expect(promise.Finalizers).To(ContainElement("kratix.io/workflows-cleanup"))
})

resources := reconcileConfigureOptsArg.Resources[0].RequiredResources
resources := reconcileConfigureOptsArg.Resources[0].GetObjects()
By("creates a service account for pipeline", func() {
Expect(resources[0]).To(BeAssignableToTypeOf(&v1.ServiceAccount{}))
sa := resources[0].(*v1.ServiceAccount)
Expect(sa.GetLabels()).To(Equal(promiseCommonLabels))
})

By("creates a config map with the promise scheduling in it", func() {
Expect(resources[3]).To(BeAssignableToTypeOf(&v1.ConfigMap{}))
configMap := resources[3].(*v1.ConfigMap)
Expect(resources[1]).To(BeAssignableToTypeOf(&v1.ConfigMap{}))
configMap := resources[1].(*v1.ConfigMap)
Expect(configMap.GetName()).To(Equal("destination-selectors-" + promise.GetName()))
Expect(configMap.GetNamespace()).To(Equal("kratix-platform-system"))
Expect(configMap.GetLabels()).To(Equal(promiseCommonLabels))
Expand All @@ -425,8 +425,8 @@ var _ = Describe("PromiseController", func() {

promiseResourcesName.Namespace = ""
By("creates a role for the pipeline service account", func() {
Expect(resources[1]).To(BeAssignableToTypeOf(&rbacv1.ClusterRole{}))
role := resources[1].(*rbacv1.ClusterRole)
Expect(resources[2]).To(BeAssignableToTypeOf(&rbacv1.ClusterRole{}))
role := resources[2].(*rbacv1.ClusterRole)
Expect(role.GetLabels()).To(Equal(promiseCommonLabels))
Expect(role.Rules).To(ConsistOf(
rbacv1.PolicyRule{
Expand All @@ -439,8 +439,8 @@ var _ = Describe("PromiseController", func() {
})

By("associates the new role with the new service account", func() {
Expect(resources[2]).To(BeAssignableToTypeOf(&rbacv1.ClusterRoleBinding{}))
binding := resources[2].(*rbacv1.ClusterRoleBinding)
Expect(resources[3]).To(BeAssignableToTypeOf(&rbacv1.ClusterRoleBinding{}))
binding := resources[3].(*rbacv1.ClusterRoleBinding)
Expect(binding.RoleRef.Name).To(Equal("promise-with-workflow-promise-configure-first-pipeline"))
Expect(binding.Subjects).To(HaveLen(1))
Expect(binding.Subjects[0]).To(Equal(rbacv1.Subject{
Expand Down
Loading

0 comments on commit 83530b2

Please sign in to comment.