diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index c19bfed5..e715c906 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -30,7 +30,6 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -68,18 +67,14 @@ type Pipeline struct { } type PipelineFactory struct { - ID string - Promise *Promise - Pipeline *Pipeline - Namespace string - - ResourceRequest *unstructured.Unstructured - CRD *apiextensionsv1.CustomResourceDefinition - + ID string + Promise *Promise + Pipeline *Pipeline + Namespace string + ResourceRequest *unstructured.Unstructured ResourceWorkflow bool - - WorkflowAction Action - WorkflowType Type + WorkflowAction Action + WorkflowType Type } // +kubebuilder:object:generate=false @@ -140,7 +135,7 @@ func (p *Pipeline) ForPromise(promise *Promise, action Action) *PipelineFactory } } -func (p *Pipeline) ForResource(promise *Promise, action Action, crd *apiextensionsv1.CustomResourceDefinition, resourceRequest *unstructured.Unstructured) *PipelineFactory { +func (p *Pipeline) ForResource(promise *Promise, action Action, resourceRequest *unstructured.Unstructured) *PipelineFactory { return &PipelineFactory{ ID: promise.GetName() + "-resource-pipeline", Promise: promise, @@ -148,7 +143,6 @@ func (p *Pipeline) ForResource(promise *Promise, action Action, crd *apiextensio ResourceRequest: resourceRequest, Namespace: resourceRequest.GetNamespace(), ResourceWorkflow: true, - CRD: crd, WorkflowType: WorkflowTypeResource, WorkflowAction: action, } @@ -163,7 +157,10 @@ func (p *PipelineFactory) Resources(jobEnv []corev1.EnvVar) (PipelineJobResource serviceAccount := p.ServiceAccount() - role := p.ObjectRole() + role, err := p.ObjectRole() + if err != nil { + return PipelineJobResources{}, err + } roleBinding := p.ObjectRoleBinding(role.GetName(), serviceAccount) job, err := p.PipelineJob(schedulingConfigMap, serviceAccount, jobEnv) @@ -193,11 +190,11 @@ func (p *PipelineFactory) ServiceAccount() *corev1.ServiceAccount { } } -func (p *PipelineFactory) ObjectRole() client.Object { +func (p *PipelineFactory) ObjectRole() (client.Object, error) { if p.ResourceWorkflow { return p.role() } - return p.clusterRole() + return p.clusterRole(), nil } func (p *PipelineFactory) ObjectRoleBinding(roleName string, serviceAccount *corev1.ServiceAccount) client.Object { @@ -481,8 +478,12 @@ func (p *PipelineFactory) getObjAndHash() (*unstructured.Unstructured, string, e return p.ResourceRequest, hash.ComputeHash(fmt.Sprintf("%s-%s", promiseHash, resourceHash)), nil } -func (p *PipelineFactory) role() *rbacv1.Role { - plural := p.CRD.Spec.Names.Plural +func (p *PipelineFactory) role() (*rbacv1.Role, error) { + crd, err := p.Promise.GetAPIAsCRD() + if err != nil { + return nil, err + } + plural := crd.Spec.Names.Plural return &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: p.ID, @@ -491,7 +492,7 @@ func (p *PipelineFactory) role() *rbacv1.Role { }, Rules: []rbacv1.PolicyRule{ { - APIGroups: []string{p.CRD.Spec.Group}, + APIGroups: []string{crd.Spec.Group}, Resources: []string{plural, plural + "/status"}, Verbs: []string{"get", "list", "update", "create", "patch"}, }, @@ -501,7 +502,7 @@ func (p *PipelineFactory) role() *rbacv1.Role { Verbs: []string{"*"}, }, }, - } + }, nil } func (p *PipelineFactory) roleBinding(roleName string, serviceAccount *corev1.ServiceAccount) *rbacv1.RoleBinding { @@ -591,7 +592,9 @@ func WorkflowLabels(workflowType Type, workflowAction Action, pipelineName strin } if workflowAction != "" { - ls = labels.Merge(ls, map[string]string{}) + ls = labels.Merge(ls, map[string]string{ + WorkActionLabel: string(workflowAction), + }) } return ls } diff --git a/api/v1alpha1/pipeline_types_test.go b/api/v1alpha1/pipeline_types_test.go index ee74bfb9..3233e67d 100644 --- a/api/v1alpha1/pipeline_types_test.go +++ b/api/v1alpha1/pipeline_types_test.go @@ -1,6 +1,7 @@ package v1alpha1_test import ( + "encoding/json" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" @@ -11,6 +12,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "strings" ) @@ -47,6 +49,17 @@ var _ = Describe("Pipeline", func() { ImagePullSecrets: []corev1.LocalObjectReference{{Name: "imagePullSecret"}}, }, } + promiseCrd = &apiextensionsv1.CustomResourceDefinition{ + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "promise.crd.group", + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "promiseCrdPlural", + }, + }, + } + + rawCrd, err := json.Marshal(promiseCrd) + Expect(err).ToNot(HaveOccurred()) promise = &v1alpha1.Promise{ TypeMeta: metav1.TypeMeta{ APIVersion: "fake.promise.group/v1", @@ -60,17 +73,10 @@ var _ = Describe("Pipeline", func() { {MatchLabels: map[string]string{"label": "value"}}, {MatchLabels: map[string]string{"another-label": "another-value"}}, }, + API: &runtime.RawExtension{Raw: rawCrd}, }, } - promiseCrd = &apiextensionsv1.CustomResourceDefinition{ - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Group: "promise.crd.group", - Names: apiextensionsv1.CustomResourceDefinitionNames{ - Plural: "promiseCrdPlural", - }, - }, - } resourceRequest = &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "fake.resource.group/v1", @@ -83,7 +89,7 @@ var _ = Describe("Pipeline", func() { }) Describe("Pipeline Factory Constructors", func() { - Describe("#ForPromise", func() { + Describe("ForPromise", func() { It("sets the appropriate fields", func() { f := pipeline.ForPromise(promise, v1alpha1.WorkflowActionConfigure) Expect(f).ToNot(BeNil()) @@ -98,9 +104,9 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ForResource", func() { + Describe("ForResource", func() { It("sets the appropriate fields", func() { - f := pipeline.ForResource(promise, v1alpha1.WorkflowActionConfigure, promiseCrd, resourceRequest) + f := pipeline.ForResource(promise, v1alpha1.WorkflowActionConfigure, resourceRequest) Expect(f).ToNot(BeNil()) Expect(f.ID).To(Equal(promise.GetName() + "-resource-pipeline")) Expect(f.Promise).To(Equal(promise)) @@ -124,7 +130,6 @@ var _ = Describe("Pipeline", func() { ID: "factoryID", Namespace: "factoryNamespace", Promise: promise, - CRD: promiseCrd, WorkflowAction: "fakeAction", WorkflowType: "fakeType", ResourceRequest: resourceRequest, @@ -132,12 +137,13 @@ var _ = Describe("Pipeline", func() { } }) - Describe("#Resources", func() { + Describe("Resources", func() { When("building resources for the configure action", func() { It("should return a list of resources", func() { factory.WorkflowAction = v1alpha1.WorkflowActionConfigure env := []corev1.EnvVar{{Name: "env1", Value: "value1"}} - role := factory.ObjectRole() + role, err := factory.ObjectRole() + Expect(err).ToNot(HaveOccurred()) serviceAccount := factory.ServiceAccount() configMap, err := factory.ConfigMap(promise.GetWorkloadGroupScheduling()) Expect(err).ToNot(HaveOccurred()) @@ -161,7 +167,8 @@ var _ = Describe("Pipeline", func() { It("should return a list of resources", func() { factory.WorkflowAction = v1alpha1.WorkflowActionDelete env := []corev1.EnvVar{{Name: "env1", Value: "value1"}} - role := factory.ObjectRole() + role, err := factory.ObjectRole() + Expect(err).ToNot(HaveOccurred()) serviceAccount := factory.ServiceAccount() configMap, err := factory.ConfigMap(promise.GetWorkloadGroupScheduling()) Expect(err).ToNot(HaveOccurred()) @@ -183,7 +190,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ServiceAccount", func() { + Describe("ServiceAccount", func() { It("should return a service account", func() { sa := factory.ServiceAccount() Expect(sa).ToNot(BeNil()) @@ -193,10 +200,11 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ObjectRole", func() { + Describe("ObjectRole", func() { When("building a role for a promise pipeline", func() { It("returns a cluster role", func() { - objectRole := factory.ObjectRole() + objectRole, err := factory.ObjectRole() + Expect(err).ToNot(HaveOccurred()) Expect(objectRole).ToNot(BeNil()) Expect(objectRole).To(BeAssignableToTypeOf(&rbacv1.ClusterRole{})) @@ -216,7 +224,8 @@ var _ = Describe("Pipeline", func() { It("returns a role", func() { factory.ResourceWorkflow = true - objectRole := factory.ObjectRole() + objectRole, err := factory.ObjectRole() + Expect(err).ToNot(HaveOccurred()) Expect(objectRole).ToNot(BeNil()) Expect(objectRole).To(BeAssignableToTypeOf(&rbacv1.Role{})) @@ -238,7 +247,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ObjectRoleBinding", func() { + Describe("ObjectRoleBinding", func() { var serviceAccount *corev1.ServiceAccount BeforeEach(func() { serviceAccount = &corev1.ServiceAccount{ @@ -300,7 +309,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ConfigMap", func() { + Describe("ConfigMap", func() { It("should return a config map", func() { workloadGroupScheduling := []v1alpha1.WorkloadGroupScheduling{ {MatchLabels: map[string]string{"label": "value"}, Source: "promise"}, @@ -316,7 +325,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#DefaultVolumes", func() { + Describe("DefaultVolumes", func() { It("should return a list of default volumes", func() { configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -339,7 +348,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#DefaultPipelineVolumes", func() { + Describe("DefaultPipelineVolumes", func() { It("should return a list of default pipeline volumes", func() { volumes, volumeMounts := factory.DefaultPipelineVolumes() Expect(volumes).To(HaveLen(3)) @@ -358,7 +367,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#DefaultEnvVars", func() { + Describe("DefaultEnvVars", func() { It("should return a list of default environment variables", func() { envVars := factory.DefaultEnvVars() Expect(envVars).To(HaveLen(3)) @@ -370,7 +379,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#ReaderContainer", func() { + Describe("ReaderContainer", func() { When("building the reader container for a promise pipeline", func() { It("returns a the reader container with the promise information", func() { container := factory.ReaderContainer() @@ -414,7 +423,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#WorkCreatorContainer", func() { + Describe("WorkCreatorContainer", func() { When("building the work creator container for a promise pipeline", func() { It("returns a the work creator container with the appropriate command", func() { expectedFlags := strings.Join([]string{ @@ -463,7 +472,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#PipelineContainers", func() { + Describe("PipelineContainers", func() { var defaultEnvVars []corev1.EnvVar var defaultVolumes []corev1.Volume var defaultVolumeMounts []corev1.VolumeMount @@ -504,7 +513,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#StatusWriterContainer", func() { + Describe("StatusWriterContainer", func() { var obj *unstructured.Unstructured var envVars []corev1.EnvVar BeforeEach(func() { @@ -546,7 +555,7 @@ var _ = Describe("Pipeline", func() { }) }) - Describe("#PipelineJob", func() { + Describe("PipelineJob", func() { var ( serviceAccount *corev1.ServiceAccount configMap *corev1.ConfigMap @@ -576,6 +585,7 @@ var _ = Describe("Pipeline", func() { Expect(definedLabels).To(SatisfyAll( HaveKeyWithValue(v1alpha1.PromiseNameLabel, promise.GetName()), HaveKeyWithValue(v1alpha1.WorkTypeLabel, string(factory.WorkflowType)), + HaveKeyWithValue(v1alpha1.WorkActionLabel, string(factory.WorkflowAction)), HaveKeyWithValue(v1alpha1.PipelineNameLabel, pipeline.GetName()), HaveKeyWithValue(v1alpha1.KratixResourceHashLabel, promiseHash(promise)), Not(HaveKey(v1alpha1.ResourceNameLabel)), @@ -660,6 +670,7 @@ var _ = Describe("Pipeline", func() { Expect(definedLabels).To(SatisfyAll( HaveKeyWithValue(v1alpha1.PromiseNameLabel, promise.GetName()), HaveKeyWithValue(v1alpha1.WorkTypeLabel, string(factory.WorkflowType)), + HaveKeyWithValue(v1alpha1.WorkActionLabel, string(factory.WorkflowAction)), HaveKeyWithValue(v1alpha1.PipelineNameLabel, pipeline.GetName()), HaveKeyWithValue(v1alpha1.KratixResourceHashLabel, combinedHash(promiseHash(promise), resourceHash(resourceRequest))), HaveKeyWithValue(v1alpha1.ResourceNameLabel, resourceRequest.GetName()), diff --git a/api/v1alpha1/promise_types.go b/api/v1alpha1/promise_types.go index 87779226..c4099ca6 100644 --- a/api/v1alpha1/promise_types.go +++ b/api/v1alpha1/promise_types.go @@ -283,7 +283,7 @@ func (p *Promise) generatePipelinesObjects(workflowType Type, workflowAction Act return nil, err } - allResources := []PipelineJobResources{} + var allResources []PipelineJobResources pipelines := promisePipelines[workflowType][workflowAction] lastIndex := len(pipelines) - 1 @@ -297,7 +297,7 @@ func (p *Promise) generatePipelinesObjects(workflowType Type, workflowAction Act var err error switch workflowType { case WorkflowTypeResource: - resources, err = pipe.ForResource(p, workflowAction, crd, resourceRequest).Resources(additionalJobEnv) + resources, err = pipe.ForResource(p, workflowAction, resourceRequest).Resources(additionalJobEnv) case WorkflowTypePromise: resources, err = pipe.ForPromise(p, workflowAction).Resources(additionalJobEnv) } diff --git a/api/v1alpha1/work_types.go b/api/v1alpha1/work_types.go index 18beccae..71b6a243 100644 --- a/api/v1alpha1/work_types.go +++ b/api/v1alpha1/work_types.go @@ -30,6 +30,7 @@ const ( ResourceNameLabel = KratixPrefix + "resource-name" PipelineNameLabel = KratixPrefix + "pipeline-name" WorkTypeLabel = KratixPrefix + "work-type" + WorkActionLabel = KratixPrefix + "work-action" WorkTypePromise = "promise" WorkTypeResource = "resource" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a9d2c4a1..8992e29c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,7 +22,6 @@ package v1alpha1 import ( "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -452,11 +451,6 @@ func (in *PipelineFactory) DeepCopyInto(out *PipelineFactory) { in, out := &in.ResourceRequest, &out.ResourceRequest *out = (*in).DeepCopy() } - if in.CRD != nil { - in, out := &in.CRD, &out.CRD - *out = new(apiextensionsv1.CustomResourceDefinition) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineFactory.