From 06a3f84d5510d93931db6add446f61855ec0e3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Moreno=20Garc=C3=ADa?= Date: Wed, 22 Feb 2023 16:49:04 +0100 Subject: [PATCH] feat(HACBS-1835): add support for workspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some requests need to be executed by Pipelines which use workspaces. This requires the PipelineRun created by the Internal Services operator to define a VolumeClaim that creates the PVC that will be attached. Signed-off-by: David Moreno García --- api/v1alpha1/internalrequest_types.go | 6 +- api/v1alpha1/internalservicesconfig_types.go | 22 ++- api/v1alpha1/zz_generated.deepcopy.go | 32 +-- ...appstudio.redhat.com_internalrequests.yaml | 7 +- ...io.redhat.com_internalservicesconfigs.yaml | 26 ++- controllers/internalrequest/adapter.go | 94 ++++++--- controllers/internalrequest/adapter_test.go | 168 ++++++++++++---- controllers/internalrequest/controller.go | 8 +- go.mod | 2 +- loader/loader.go | 24 ++- loader/loader_mock.go | 97 +++++++++ loader/loader_mock_test.go | 134 +++++++++++++ loader/loader_suite_test.go | 3 +- loader/loader_test.go | 22 ++- tekton/pipeline_run.go | 117 +++++------ tekton/pipeline_run_test.go | 185 +++++++++--------- 16 files changed, 688 insertions(+), 259 deletions(-) create mode 100644 loader/loader_mock.go create mode 100644 loader/loader_mock_test.go diff --git a/api/v1alpha1/internalrequest_types.go b/api/v1alpha1/internalrequest_types.go index 92ab7b7..fdda3df 100644 --- a/api/v1alpha1/internalrequest_types.go +++ b/api/v1alpha1/internalrequest_types.go @@ -22,7 +22,7 @@ import ( "time" ) -// HandlingReason represents a reason for the release "InternalRequestSucceeded" condition. +// HandlingReason represents a reason for the InternalRequest "InternalRequestSucceeded" condition. type HandlingReason string const ( @@ -61,11 +61,11 @@ type InternalRequestSpec struct { // InternalRequestStatus defines the observed state of InternalRequest. type InternalRequestStatus struct { - // StartTime is the time when the Release PipelineRun was created and set to run + // StartTime is the time when the InternalRequest PipelineRun was created and set to run // +optional StartTime *metav1.Time `json:"startTime,omitempty"` - // CompletionTime is the time the Release PipelineRun completed + // CompletionTime is the time the InternalRequest PipelineRun completed // +optional CompletionTime *metav1.Time `json:"completionTime,omitempty"` diff --git a/api/v1alpha1/internalservicesconfig_types.go b/api/v1alpha1/internalservicesconfig_types.go index 5cebe33..e482f63 100644 --- a/api/v1alpha1/internalservicesconfig_types.go +++ b/api/v1alpha1/internalservicesconfig_types.go @@ -28,20 +28,26 @@ type InternalServicesConfigSpec struct { // +required AllowList []string `json:"allowList,omitempty"` - // Catalog holds the information about the Tekton catalog + // Debug sets the operator to run in debug mode. In this mode, PipelineRuns and PVCs will not be removed // +optional - Catalog Catalog `json:"catalog,omitempty"` + Debug bool `json:"debug,omitempty"` + + // VolumeClaim holds information about the volume to request for Pipelines requiring a workspace + // +kubebuilder:default={name:"workspace", size:"1Gi"} + VolumeClaim VolumeClaim `json:"volumeClaim,omitempty"` } -type Catalog struct { - // Namespace where to find the Tekton Pipelines +type VolumeClaim struct { + // Name is the workspace name // +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + // +kubebuilder:default="workspace" // +optional - Namespace string `json:"namespace,omitempty"` + Name string `json:"name,omitempty"` - // Bundle where to find the Tekton Pipelines - // +optional - Bundle string `json:"bundle,omitempty"` + // Size is the size that will be requested when a workspace is required by a Pipeline + // +kubebuilder:validation:Pattern=^[1-9][0-9]*(K|M|G)i$ + // +kubebuilder:default="1Gi" + Size string `json:"size,omitempty"` } // InternalServicesConfigStatus defines the observed state of InternalServicesConfig. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 48bc9f2..6d5a959 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -26,21 +26,6 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Catalog) DeepCopyInto(out *Catalog) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Catalog. -func (in *Catalog) DeepCopy() *Catalog { - if in == nil { - return nil - } - out := new(Catalog) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalRequest) DeepCopyInto(out *InternalRequest) { *out = *in @@ -226,7 +211,7 @@ func (in *InternalServicesConfigSpec) DeepCopyInto(out *InternalServicesConfigSp *out = make([]string, len(*in)) copy(*out, *in) } - out.Catalog = in.Catalog + out.VolumeClaim = in.VolumeClaim } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InternalServicesConfigSpec. @@ -253,3 +238,18 @@ func (in *InternalServicesConfigStatus) DeepCopy() *InternalServicesConfigStatus in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VolumeClaim) DeepCopyInto(out *VolumeClaim) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeClaim. +func (in *VolumeClaim) DeepCopy() *VolumeClaim { + if in == nil { + return nil + } + out := new(VolumeClaim) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/appstudio.redhat.com_internalrequests.yaml b/config/crd/bases/appstudio.redhat.com_internalrequests.yaml index 108198c..374b5c0 100644 --- a/config/crd/bases/appstudio.redhat.com_internalrequests.yaml +++ b/config/crd/bases/appstudio.redhat.com_internalrequests.yaml @@ -60,7 +60,8 @@ spec: description: InternalRequestStatus defines the observed state of InternalRequest. properties: completionTime: - description: CompletionTime is the time the Release PipelineRun completed + description: CompletionTime is the time the InternalRequest PipelineRun + completed format: date-time type: string conditions: @@ -140,8 +141,8 @@ spec: Tekton PipelineRun kubebuilder:pruning:PreserveUnknownFields type: object startTime: - description: StartTime is the time when the Release PipelineRun was - created and set to run + description: StartTime is the time when the InternalRequest PipelineRun + was created and set to run format: date-time type: string type: object diff --git a/config/crd/bases/appstudio.redhat.com_internalservicesconfigs.yaml b/config/crd/bases/appstudio.redhat.com_internalservicesconfigs.yaml index dd6716f..83ecd6e 100644 --- a/config/crd/bases/appstudio.redhat.com_internalservicesconfigs.yaml +++ b/config/crd/bases/appstudio.redhat.com_internalservicesconfigs.yaml @@ -42,16 +42,28 @@ spec: items: type: string type: array - catalog: - description: Catalog holds the information about the Tekton catalog + debug: + description: Debug sets the operator to run in debug mode. In this + mode, PipelineRuns and PVCs will not be removed + type: boolean + volumeClaim: + default: + name: workspace + size: 1Gi + description: VolumeClaim holds information about the volume to request + for Pipelines requiring a workspace properties: - bundle: - description: Bundle where to find the Tekton Pipelines - type: string - namespace: - description: Namespace where to find the Tekton Pipelines + name: + default: workspace + description: Name is the workspace name pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ type: string + size: + default: 1Gi + description: Size is the size that will be requested when a workspace + is required by a Pipeline + pattern: ^[1-9][0-9]*(K|M|G)i$ + type: string type: object type: object status: diff --git a/controllers/internalrequest/adapter.go b/controllers/internalrequest/adapter.go index 99be104..59381a1 100644 --- a/controllers/internalrequest/adapter.go +++ b/controllers/internalrequest/adapter.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "github.com/go-logr/logr" - libhandler "github.com/operator-framework/operator-lib/handler" "github.com/redhat-appstudio/internal-services/api/v1alpha1" "github.com/redhat-appstudio/internal-services/loader" "github.com/redhat-appstudio/internal-services/tekton" @@ -31,18 +30,18 @@ import ( "knative.dev/pkg/apis" "os" "sigs.k8s.io/controller-runtime/pkg/client" - "strings" ) // Adapter holds the objects needed to reconcile an InternalRequest. type Adapter struct { - client client.Client - internalServicesConfig *v1alpha1.InternalServicesConfig - ctx context.Context - internalClient client.Client - internalRequest *v1alpha1.InternalRequest - loader loader.ObjectLoader - logger logr.Logger + client client.Client + internalServicesConfig *v1alpha1.InternalServicesConfig + ctx context.Context + internalClient client.Client + internalRequest *v1alpha1.InternalRequest + internalRequestPipeline *v1beta1.Pipeline + loader loader.ObjectLoader + logger logr.Logger } // NewAdapter creates and returns an Adapter instance. @@ -59,6 +58,8 @@ func NewAdapter(ctx context.Context, client, internalClient client.Client, inter // EnsureConfigIsLoaded is an operation that will load the service InternalServicesConfig from the manager namespace. If not found, // a new InternalServicesConfig resource will be generated and attached to the adapter. +// +// Note: This operation sets values in the adapter to be used by other operations, so it should be always enabled. func (a *Adapter) EnsureConfigIsLoaded() (reconciler.OperationResult, error) { namespace := os.Getenv("SERVICE_NAMESPACE") if namespace == "" { @@ -78,6 +79,25 @@ func (a *Adapter) EnsureConfigIsLoaded() (reconciler.OperationResult, error) { return reconciler.ContinueProcessing() } +// EnsurePipelineExists is an operation that will ensure the Pipeline referenced by the InternalRequest exists and add it +// to the adapter, so it can be used in other operations. If the Pipeline doesn't exist, the InternalRequest will be +// marked as failed. +// +// Note: This operation sets values in the adapter to be used by other operations, so it should be always enabled. +func (a *Adapter) EnsurePipelineExists() (reconciler.OperationResult, error) { + var err error + a.internalRequestPipeline, err = a.loader.GetInternalRequestPipeline(a.ctx, a.internalClient, + a.internalRequest.Spec.Request, a.internalServicesConfig.Namespace) + + if err != nil { + patch := client.MergeFrom(a.internalRequest.DeepCopy()) + a.internalRequest.MarkFailed(fmt.Sprintf("No endpoint to handle '%s' requests", a.internalRequest.Spec.Request)) + return reconciler.RequeueOnErrorOrStop(a.client.Status().Patch(a.ctx, a.internalRequest, patch)) + } + + return reconciler.ContinueProcessing() +} + // EnsurePipelineRunIsCreated is an operation that will ensure that the InternalRequest is handled by creating a new // PipelineRun for the Pipeline referenced in the Request field. func (a *Adapter) EnsurePipelineRunIsCreated() (reconciler.OperationResult, error) { @@ -88,14 +108,7 @@ func (a *Adapter) EnsurePipelineRunIsCreated() (reconciler.OperationResult, erro if pipelineRun == nil || !a.internalRequest.HasStarted() { if pipelineRun == nil { - pipelineRun = tekton.NewPipelineRun(a.internalRequest, a.internalServicesConfig) - - err = libhandler.SetOwnerAnnotations(a.internalRequest, pipelineRun) - if err != nil { - return reconciler.RequeueWithError(err) - } - - err = a.internalClient.Create(a.ctx, pipelineRun) + pipelineRun, err = a.createInternalRequestPipelineRun() if err != nil { return reconciler.RequeueWithError(err) } @@ -110,6 +123,27 @@ func (a *Adapter) EnsurePipelineRunIsCreated() (reconciler.OperationResult, erro return reconciler.ContinueProcessing() } +// EnsurePipelineRunIsDeleted is an operation that will ensure that the PipelineRun created to handle the InternalRequest +// is deleted once it finishes. +func (a *Adapter) EnsurePipelineRunIsDeleted() (reconciler.OperationResult, error) { + if !a.internalRequest.HasCompleted() { + return reconciler.ContinueProcessing() + } + + if a.internalServicesConfig.Spec.Debug { + a.logger.Info("Running in debug mode. Skipping PipelineRun deletion") + + return reconciler.ContinueProcessing() + } + + pipelineRun, err := a.loader.GetInternalRequestPipelineRun(a.ctx, a.internalClient, a.internalRequest) + if err != nil { + return reconciler.RequeueWithError(err) + } + + return reconciler.RequeueOnErrorOrContinue(a.internalClient.Delete(a.ctx, pipelineRun)) +} + // EnsureRequestIsAllowed is an operation that will ensure that the request is coming from a namespace allowed // to execute InternalRequests. If the InternalServicesConfig spec.allowList is empty, any request will be allowed regardless of the // remote namespace. @@ -126,7 +160,7 @@ func (a *Adapter) EnsureRequestIsAllowed() (reconciler.OperationResult, error) { return reconciler.RequeueOnErrorOrStop(a.client.Status().Patch(a.ctx, a.internalRequest, patch)) } -// EnsureStatusIsTracked is an operation that will ensure that the release PipelineRun status is tracked +// EnsureStatusIsTracked is an operation that will ensure that the InternalRequest PipelineRun status is tracked // in the InternalRequest being processed. func (a *Adapter) EnsureStatusIsTracked() (reconciler.OperationResult, error) { pipelineRun, err := a.loader.GetInternalRequestPipelineRun(a.ctx, a.internalClient, a.internalRequest) @@ -141,6 +175,24 @@ func (a *Adapter) EnsureStatusIsTracked() (reconciler.OperationResult, error) { return reconciler.ContinueProcessing() } +// createInternalRequestPipelineRun creates and returns a new InternalRequest PipelineRun. The new PipelineRun will +// include owner annotations, so it triggers InternalRequest reconciles whenever it changes. The Pipeline information +// and its parameters will be extracted from the InternalRequest. +func (a *Adapter) createInternalRequestPipelineRun() (*v1beta1.PipelineRun, error) { + pipelineRun := tekton.NewInternalRequestPipelineRun(a.internalServicesConfig). + WithInternalRequest(a.internalRequest). + WithOwner(a.internalRequest). + WithPipeline(a.internalRequestPipeline, a.internalServicesConfig). + AsPipelineRun() + + err := a.internalClient.Create(a.ctx, pipelineRun) + if err != nil { + return nil, err + } + + return pipelineRun, nil +} + // getDefaultInternalServicesConfig creates and returns a InternalServicesConfig resource in the given namespace with default values. func (a *Adapter) getDefaultInternalServicesConfig(namespace string) *v1alpha1.InternalServicesConfig { return &v1alpha1.InternalServicesConfig{ @@ -176,12 +228,6 @@ func (a *Adapter) registerInternalRequestPipelineRunStatus(pipelineRun *v1beta1. if condition.IsTrue() { a.internalRequest.Status.Results = tekton.GetResultsFromPipelineRun(pipelineRun) a.internalRequest.MarkSucceeded() - } else if strings.Contains(condition.Message, "not found") { - var endpoint string - if pipelineRun.Spec.PipelineRef != nil { - endpoint = pipelineRun.Spec.PipelineRef.Name - } - a.internalRequest.MarkFailed(fmt.Sprintf("No endpoint to handle '%s' requests", endpoint)) } else { a.internalRequest.MarkFailed(condition.Message) } diff --git a/controllers/internalrequest/adapter_test.go b/controllers/internalrequest/adapter_test.go index 22aec8c..be73fb7 100644 --- a/controllers/internalrequest/adapter_test.go +++ b/controllers/internalrequest/adapter_test.go @@ -36,7 +36,8 @@ var _ = Describe("PipelineRun", Ordered, func() { createResources func() deleteResources func() - adapter *Adapter + adapter *Adapter + pipeline *tektonv1beta1.Pipeline ) Context("When calling NewAdapter", func() { @@ -73,7 +74,7 @@ var _ = Describe("PipelineRun", Ordered, func() { }) }) - Context("When calling EnsurePipelineRunIsCreated", func() { + Context("When calling EnsurePipelineExists", func() { AfterEach(func() { deleteResources() }) @@ -82,27 +83,46 @@ var _ = Describe("PipelineRun", Ordered, func() { createResources() }) - It("ensures a PipelineRun exists", func() { - result, err := adapter.EnsurePipelineRunIsCreated() - Expect(!result.CancelRequest && err == nil).Should(BeTrue()) + It("should continue if the Pipeline exists", func() { + result, err := adapter.EnsurePipelineExists() + Expect(!result.CancelRequest && !result.RequeueRequest).To(BeTrue()) + Expect(err).To(BeNil()) + }) - pipelineRun, err := adapter.loader.GetInternalRequestPipelineRun(ctx, k8sClient, adapter.internalRequest) - Expect(pipelineRun).ToNot(BeNil()) - Expect(err).ToNot(HaveOccurred()) + It("should set a 'No endpoint to handle' if the Pipeline was not found", func() { + adapter.ctx = loader.GetMockedContext(ctx, []loader.MockData{ + { + ContextKey: loader.InternalRequestPipelineContextKey, + Err: fmt.Errorf("not found"), + }, + }) + + result, err := adapter.EnsurePipelineExists() + Expect(result.CancelRequest && !result.RequeueRequest).To(BeTrue()) + Expect(err).To(BeNil()) + + condition := meta.FindStatusCondition(adapter.internalRequest.Status.Conditions, v1alpha1.InternalRequestSucceededConditionType) + Expect(condition.Message).To(Equal(fmt.Sprintf("No endpoint to handle '%s' requests", adapter.internalRequest.Spec.Request))) + }) + }) + + Context("When calling EnsurePipelineRunIsCreated", func() { + AfterEach(func() { + deleteResources() + }) + + BeforeEach(func() { + createResources() + adapter.internalRequestPipeline = pipeline }) - It("ensures the pipelineRun is owned by the InternalRequest", func() { + It("ensures a PipelineRun exists", func() { result, err := adapter.EnsurePipelineRunIsCreated() Expect(!result.CancelRequest && err == nil).Should(BeTrue()) pipelineRun, err := adapter.loader.GetInternalRequestPipelineRun(ctx, k8sClient, adapter.internalRequest) Expect(pipelineRun).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) - Expect(pipelineRun.Annotations).To(HaveLen(2)) - Expect(pipelineRun.Annotations[libhandler.NamespacedNameAnnotation]).To( - Equal(adapter.internalRequest.Namespace + "/" + adapter.internalRequest.Name), - ) - Expect(pipelineRun.Annotations[libhandler.TypeAnnotation]).To(Equal(adapter.internalRequest.Kind)) }) It("ensures the InternalRequest is marked as running", func() { @@ -112,6 +132,56 @@ var _ = Describe("PipelineRun", Ordered, func() { }) }) + Context("When calling EnsurePipelineRunIsDeleted", func() { + AfterEach(func() { + deleteResources() + }) + + BeforeEach(func() { + createResources() + }) + + It("should continue if the InternalRequest has not completed", func() { + result, err := adapter.EnsurePipelineRunIsDeleted() + Expect(!result.CancelRequest && !result.RequeueRequest).To(BeTrue()) + Expect(err).To(BeNil()) + }) + + It("should continue if the operator is set to run in debug mode", func() { + adapter.internalServicesConfig.Spec.Debug = true + result, err := adapter.EnsurePipelineRunIsDeleted() + Expect(!result.CancelRequest && !result.RequeueRequest).To(BeTrue()) + Expect(err).To(BeNil()) + }) + + It("should requeue if it fails to load the InternalRequest PipelineRun", func() { + adapter.ctx = loader.GetMockedContext(ctx, []loader.MockData{ + { + ContextKey: loader.InternalRequestPipelineRunContextKey, + Err: fmt.Errorf("not found"), + }, + }) + adapter.internalRequest.MarkSucceeded() + + result, err := adapter.EnsurePipelineRunIsDeleted() + Expect(!result.CancelRequest && result.RequeueRequest).To(BeTrue()) + Expect(err).NotTo(BeNil()) + }) + + It("should delete the InternalRequest PipelineRun", func() { + adapter.internalRequestPipeline = pipeline + adapter.internalRequest.MarkSucceeded() + + pipelineRun, err := adapter.createInternalRequestPipelineRun() + Expect(pipelineRun).NotTo(BeNil()) + Expect(err).NotTo(HaveOccurred()) + + result, err := adapter.EnsurePipelineRunIsDeleted() + Expect(!result.CancelRequest && !result.RequeueRequest).To(BeTrue()) + Expect(err).To(BeNil()) + }) + }) + Context("When calling EnsureRequestIsAllowed", func() { AfterEach(func() { deleteResources() @@ -180,6 +250,43 @@ var _ = Describe("PipelineRun", Ordered, func() { }) }) + Context("When calling createInternalRequestPipelineRun", func() { + AfterEach(func() { + deleteResources() + }) + + BeforeEach(func() { + createResources() + adapter.internalRequestPipeline = pipeline + }) + + It("creates a PipelineRun with the InternalRequest params and labels", func() { + pipelineRun, err := adapter.createInternalRequestPipelineRun() + Expect(pipelineRun).NotTo(BeNil()) + Expect(err).To(BeNil()) + Expect(pipelineRun.Labels).To(HaveLen(2)) + Expect(pipelineRun.Spec.Params).To(HaveLen(len(adapter.internalRequest.Spec.Params))) + }) + + It("creates a PipelineRun owned by the InternalRequest", func() { + pipelineRun, err := adapter.createInternalRequestPipelineRun() + Expect(pipelineRun).NotTo(BeNil()) + Expect(err).To(BeNil()) + Expect(pipelineRun.Annotations).To(HaveLen(2)) + Expect(pipelineRun.Annotations[libhandler.NamespacedNameAnnotation]).To( + Equal(adapter.internalRequest.Namespace + "/" + adapter.internalRequest.Name), + ) + Expect(pipelineRun.Annotations[libhandler.TypeAnnotation]).To(Equal(adapter.internalRequest.Kind)) + }) + + It("creates a PipelineRun referencing the Pipeline requested in the InternalRequest", func() { + pipelineRun, err := adapter.createInternalRequestPipelineRun() + Expect(pipelineRun).NotTo(BeNil()) + Expect(err).To(BeNil()) + Expect(pipelineRun.Spec.PipelineRef.Name).To(Equal(adapter.internalRequestPipeline.Name)) + }) + }) + Context("When calling getDefaultInternalServicesConfig", func() { AfterEach(func() { deleteResources() @@ -275,32 +382,12 @@ var _ = Describe("PipelineRun", Ordered, func() { Expect(adapter.internalRequest.HasSucceeded()).To(BeTrue()) }) - It("should set the Release as failed if the PipelineRun failed", func() { + It("should set the InternalRequest as failed if the PipelineRun failed", func() { pipelineRun := &tektonv1beta1.PipelineRun{} pipelineRun.Status.MarkFailed("", "") Expect(adapter.registerInternalRequestPipelineRunStatus(pipelineRun)).To(BeNil()) Expect(adapter.internalRequest.HasSucceeded()).To(BeFalse()) }) - - It("should set a 'No endpoint to handle' if the Pipeline was not found", func() { - pipelineRun := &tektonv1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pipeline-run", - Namespace: "default", - }, - Spec: tektonv1beta1.PipelineRunSpec{ - PipelineRef: &tektonv1beta1.PipelineRef{ - Name: "not-found", - }, - }, - } - pipelineRun.Status.MarkFailed("", "", "not found") - Expect(adapter.registerInternalRequestPipelineRunStatus(pipelineRun)).To(BeNil()) - Expect(adapter.internalRequest.HasSucceeded()).To(BeFalse()) - - condition := meta.FindStatusCondition(adapter.internalRequest.Status.Conditions, v1alpha1.InternalRequestSucceededConditionType) - Expect(condition.Message).To(Equal(fmt.Sprintf("No endpoint to handle '%s' requests", pipelineRun.Spec.PipelineRef.Name))) - }) }) createResources = func() { @@ -328,13 +415,22 @@ var _ = Describe("PipelineRun", Ordered, func() { } Expect(k8sClient.Create(ctx, internalServicesConfig)).To(Succeed()) - adapter = NewAdapter(ctx, k8sClient, k8sClient, internalRequest, loader.NewLoader(), ctrl.Log) + pipeline = &tektonv1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: internalRequest.Spec.Request, + Namespace: "default", + }, + } + Expect(k8sClient.Create(ctx, pipeline)).To(Succeed()) + + adapter = NewAdapter(ctx, k8sClient, k8sClient, internalRequest, loader.NewMockLoader(), ctrl.Log) adapter.internalServicesConfig = internalServicesConfig } deleteResources = func() { _ = k8sClient.Delete(ctx, adapter.internalServicesConfig) Expect(k8sClient.Delete(ctx, adapter.internalRequest)).To(Succeed()) + Expect(k8sClient.Delete(ctx, pipeline)).To(Succeed()) err := k8sClient.DeleteAllOf(ctx, &tektonv1beta1.PipelineRun{}) Expect(err == nil || errors.IsNotFound(err)).To(BeTrue()) } diff --git a/controllers/internalrequest/controller.go b/controllers/internalrequest/controller.go index 96ef5a8..235c75c 100644 --- a/controllers/internalrequest/controller.go +++ b/controllers/internalrequest/controller.go @@ -81,10 +81,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu adapter := NewAdapter(ctx, r.Client, r.InternalClient, internalRequest, loader.NewLoader(), logger) return reconciler.ReconcileHandler([]reconciler.ReconcileOperation{ - adapter.EnsureConfigIsLoaded, + adapter.EnsureConfigIsLoaded, // This operation sets the config in the adapter to be used in other operations. adapter.EnsureRequestIsAllowed, + adapter.EnsurePipelineExists, // This operation sets the pipeline in the adapter to be used in other operations. adapter.EnsurePipelineRunIsCreated, adapter.EnsureStatusIsTracked, + adapter.EnsurePipelineRunIsDeleted, }) } @@ -93,8 +95,8 @@ func SetupController(mgr ctrl.Manager, remoteCluster cluster.Cluster, log *logr. return setupControllerWithManager(mgr, remoteCluster, NewInternalRequestReconciler(mgr.GetClient(), remoteCluster.GetClient(), log, mgr.GetScheme())) } -// setupControllerWithManager sets up the controller with the Manager which monitors new Releases and filters out -// status updates. This controller also watches for PipelineRuns created by this controller and owned by the Releases so +// setupControllerWithManager sets up the controller with the Manager which monitors new InternalRequests and filters out +// status updates. This controller also watches for PipelineRuns created by this controller and owned by the InternalRequests so // the owner gets reconciled on PipelineRun changes. func setupControllerWithManager(mgr ctrl.Manager, remoteCluster cluster.Cluster, reconciler *Reconciler) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/go.mod b/go.mod index 739576c..4d5b416 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/operator-framework/operator-lib v0.11.0 github.com/redhat-appstudio/operator-goodies v0.0.0-20230113090719-a8a43f600367 github.com/tektoncd/pipeline v0.42.0 + k8s.io/api v0.25.4 k8s.io/apimachinery v0.25.4 k8s.io/client-go v0.25.4 knative.dev/pkg v0.0.0-20221011175852-714b7630a836 @@ -89,7 +90,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/api v0.25.4 // indirect k8s.io/apiextensions-apiserver v0.25.2 // indirect k8s.io/component-base v0.25.2 // indirect k8s.io/klog/v2 v2.80.1 // indirect diff --git a/loader/loader.go b/loader/loader.go index 4b2ee93..ea018a7 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -11,7 +11,8 @@ import ( type ObjectLoader interface { GetInternalRequest(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalRequest, error) - GetInternalRequestPipelineRun(ctx context.Context, cli client.Client, release *v1alpha1.InternalRequest) (*v1beta1.PipelineRun, error) + GetInternalRequestPipeline(ctx context.Context, cli client.Client, name, namespace string) (*v1beta1.Pipeline, error) + GetInternalRequestPipelineRun(ctx context.Context, cli client.Client, internalRequest *v1alpha1.InternalRequest) (*v1beta1.PipelineRun, error) GetInternalServicesConfig(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalServicesConfig, error) } @@ -30,13 +31,6 @@ func getObject(name, namespace string, cli client.Client, ctx context.Context, o }, object) } -// GetInternalServicesConfig returns the InternalServicesConfig with the given name and namespace. If the -// InternalServicesConfig is not found or the Get operation fails, an error will be returned. -func (l *loader) GetInternalServicesConfig(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalServicesConfig, error) { - internalServicesConfig := &v1alpha1.InternalServicesConfig{} - return internalServicesConfig, getObject(name, namespace, cli, ctx, internalServicesConfig) -} - // GetInternalRequest returns the InternalRequest with the given name and namespace. If the InternalRequest is not // found or the Get operation fails, an error will be returned. func (l *loader) GetInternalRequest(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalRequest, error) { @@ -44,6 +38,13 @@ func (l *loader) GetInternalRequest(ctx context.Context, cli client.Client, name return internalRequest, getObject(name, namespace, cli, ctx, internalRequest) } +// GetInternalRequestPipeline returns the Pipeline with the given name and namespace. If the Pipeline is not +// found or the Get operation fails, an error will be returned. +func (l *loader) GetInternalRequestPipeline(ctx context.Context, cli client.Client, name, namespace string) (*v1beta1.Pipeline, error) { + pipeline := &v1beta1.Pipeline{} + return pipeline, getObject(name, namespace, cli, ctx, pipeline) +} + // GetInternalRequestPipelineRun returns the PipelineRun referenced by the given InternalRequest or nil if it's not // found. In the case the List operation fails, an error will be returned. func (l *loader) GetInternalRequestPipelineRun(ctx context.Context, cli client.Client, internalRequest *v1alpha1.InternalRequest) (*v1beta1.PipelineRun, error) { @@ -61,3 +62,10 @@ func (l *loader) GetInternalRequestPipelineRun(ctx context.Context, cli client.C return nil, err } + +// GetInternalServicesConfig returns the InternalServicesConfig with the given name and namespace. If the +// InternalServicesConfig is not found or the Get operation fails, an error will be returned. +func (l *loader) GetInternalServicesConfig(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalServicesConfig, error) { + internalServicesConfig := &v1alpha1.InternalServicesConfig{} + return internalServicesConfig, getObject(name, namespace, cli, ctx, internalServicesConfig) +} diff --git a/loader/loader_mock.go b/loader/loader_mock.go new file mode 100644 index 0000000..2b89352 --- /dev/null +++ b/loader/loader_mock.go @@ -0,0 +1,97 @@ +package loader + +import ( + "context" + "github.com/redhat-appstudio/internal-services/api/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type ( + contextKey int + MockData struct { + ContextKey contextKey + Err error + Resource any + } + mockLoader struct { + loader ObjectLoader + } +) + +const ( + InternalRequestContextKey contextKey = iota + InternalRequestPipelineContextKey contextKey = iota + InternalRequestPipelineRunContextKey contextKey = iota + InternalServicesConfigContextKey contextKey = iota +) + +func GetMockedContext(ctx context.Context, data []MockData) context.Context { + for _, mockData := range data { + ctx = context.WithValue(ctx, mockData.ContextKey, mockData) + } + + return ctx +} + +func NewMockLoader() ObjectLoader { + return &mockLoader{ + loader: NewLoader(), + } +} + +// getMockedResourceAndErrorFromContext returns the mocked data found in the context passed as an argument. The data is +// to be found in the contextDataKey key. If not there, a panic will be raised. +func getMockedResourceAndErrorFromContext[T any](ctx context.Context, contextKey contextKey, _ T) (T, error) { + var resource T + var err error + + value := ctx.Value(contextKey) + if value == nil { + panic("Mocked data not found in the context") + } + + data, _ := value.(MockData) + + if data.Resource != nil { + resource = data.Resource.(T) + } + + if data.Err != nil { + err = data.Err + } + + return resource, err +} + +// GetInternalRequest returns the resource and error passed as values of the context. +func (l *mockLoader) GetInternalRequest(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalRequest, error) { + if ctx.Value(InternalRequestContextKey) == nil { + return l.loader.GetInternalRequest(ctx, cli, name, namespace) + } + return getMockedResourceAndErrorFromContext(ctx, InternalRequestContextKey, &v1alpha1.InternalRequest{}) +} + +// GetInternalRequestPipeline returns the resource and error passed as values of the context. +func (l *mockLoader) GetInternalRequestPipeline(ctx context.Context, cli client.Client, name, namespace string) (*v1beta1.Pipeline, error) { + if ctx.Value(InternalRequestPipelineContextKey) == nil { + return l.loader.GetInternalRequestPipeline(ctx, cli, name, namespace) + } + return getMockedResourceAndErrorFromContext(ctx, InternalRequestPipelineContextKey, &v1beta1.Pipeline{}) +} + +// GetInternalRequestPipelineRun returns the resource and error passed as values of the context. +func (l *mockLoader) GetInternalRequestPipelineRun(ctx context.Context, cli client.Client, internalRequest *v1alpha1.InternalRequest) (*v1beta1.PipelineRun, error) { + if ctx.Value(InternalRequestPipelineRunContextKey) == nil { + return l.loader.GetInternalRequestPipelineRun(ctx, cli, internalRequest) + } + return getMockedResourceAndErrorFromContext(ctx, InternalRequestPipelineRunContextKey, &v1beta1.PipelineRun{}) +} + +// GetInternalServicesConfig returns the resource and error passed as values of the context. +func (l *mockLoader) GetInternalServicesConfig(ctx context.Context, cli client.Client, name, namespace string) (*v1alpha1.InternalServicesConfig, error) { + if ctx.Value(InternalServicesConfigContextKey) == nil { + return l.loader.GetInternalServicesConfig(ctx, cli, name, namespace) + } + return getMockedResourceAndErrorFromContext(ctx, InternalServicesConfigContextKey, &v1alpha1.InternalServicesConfig{}) +} diff --git a/loader/loader_mock_test.go b/loader/loader_mock_test.go new file mode 100644 index 0000000..19bfa9b --- /dev/null +++ b/loader/loader_mock_test.go @@ -0,0 +1,134 @@ +package loader + +import ( + "errors" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/redhat-appstudio/internal-services/api/v1alpha1" + tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + v12 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("Loader Mock", Ordered, func() { + var ( + loader ObjectLoader + ) + + BeforeAll(func() { + loader = NewMockLoader() + }) + + Context("When calling getMockedResourceAndErrorFromContext", func() { + contextErr := errors.New("error") + contextResource := &v1alpha1.InternalRequest{ + ObjectMeta: v12.ObjectMeta{ + Name: "pod", + Namespace: "default", + }, + } + + It("returns the resource from the context", func() { + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestContextKey, + Resource: contextResource, + }, + }) + resource, err := getMockedResourceAndErrorFromContext(mockContext, InternalRequestContextKey, contextResource) + Expect(err).To(BeNil()) + Expect(resource).To(Equal(contextResource)) + }) + + It("returns the error from the context", func() { + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestContextKey, + Err: contextErr, + }, + }) + resource, err := getMockedResourceAndErrorFromContext(mockContext, InternalRequestContextKey, contextResource) + Expect(err).To(Equal(contextErr)) + Expect(resource).To(BeNil()) + }) + + It("returns the resource and the error from the context", func() { + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestContextKey, + Resource: contextResource, + Err: contextErr, + }, + }) + resource, err := getMockedResourceAndErrorFromContext(mockContext, InternalRequestContextKey, contextResource) + Expect(err).To(Equal(contextErr)) + Expect(resource).To(Equal(contextResource)) + }) + + It("should panic when the mocked data is not present", func() { + Expect(func() { + _, _ = getMockedResourceAndErrorFromContext(ctx, InternalRequestContextKey, contextResource) + }).To(Panic()) + }) + }) + + Context("When calling GetInternalRequest", func() { + It("returns the resource and error from the context", func() { + internalRequest := &v1alpha1.InternalRequest{} + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestContextKey, + Resource: internalRequest, + }, + }) + resource, err := loader.GetInternalRequest(mockContext, nil, "", "") + Expect(resource).To(Equal(internalRequest)) + Expect(err).To(BeNil()) + }) + }) + + Context("When calling GetInternalRequestPipeline", func() { + It("returns the resource and error from the context", func() { + pipeline := &tektonv1beta1.Pipeline{} + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestPipelineContextKey, + Resource: pipeline, + }, + }) + resource, err := loader.GetInternalRequestPipeline(mockContext, nil, "", "") + Expect(resource).To(Equal(pipeline)) + Expect(err).To(BeNil()) + }) + }) + + Context("When calling GetInternalRequestPipelineRun", func() { + It("returns the resource and error from the context", func() { + pipelineRun := &tektonv1beta1.PipelineRun{} + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalRequestPipelineRunContextKey, + Resource: pipelineRun, + }, + }) + resource, err := loader.GetInternalRequestPipelineRun(mockContext, nil, &v1alpha1.InternalRequest{}) + Expect(resource).To(Equal(pipelineRun)) + Expect(err).To(BeNil()) + }) + }) + + Context("When calling GetInternalServicesConfig", func() { + It("returns the resource and error from the context", func() { + internalServicesConfig := &v1alpha1.InternalServicesConfig{} + mockContext := GetMockedContext(ctx, []MockData{ + { + ContextKey: InternalServicesConfigContextKey, + Resource: internalServicesConfig, + }, + }) + resource, err := loader.GetInternalServicesConfig(mockContext, nil, "", "") + Expect(resource).To(Equal(internalServicesConfig)) + Expect(err).To(BeNil()) + }) + }) + +}) diff --git a/loader/loader_suite_test.go b/loader/loader_suite_test.go index 52e24b9..1f8710d 100644 --- a/loader/loader_suite_test.go +++ b/loader/loader_suite_test.go @@ -87,7 +87,8 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) - k8sClient = mgr.GetClient() + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) go func() { defer GinkgoRecover() Expect(mgr.Start(ctx)).To(Succeed()) diff --git a/loader/loader_test.go b/loader/loader_test.go index 10672ae..5cbd9d6 100644 --- a/loader/loader_test.go +++ b/loader/loader_test.go @@ -17,6 +17,7 @@ var _ = Describe("Loader", Ordered, func() { internalServicesConfig *v1alpha1.InternalServicesConfig internalRequest *v1alpha1.InternalRequest + pipeline *tektonv1beta1.Pipeline pipelineRun *tektonv1beta1.PipelineRun ) @@ -52,15 +53,24 @@ var _ = Describe("Loader", Ordered, func() { }) }) + Context("When calling GetInternalRequestPipeline", func() { + It("returns the requested Pipeline", func() { + returnedObject, err := loader.GetInternalRequestPipeline(ctx, k8sClient, pipeline.Name, pipeline.Namespace) + Expect(err).NotTo(HaveOccurred()) + Expect(returnedObject).NotTo(Equal(&tektonv1beta1.Pipeline{})) + Expect(returnedObject.Name).To(Equal(pipeline.Name)) + }) + }) + Context("When calling GetInternalRequestPipelineRun", func() { - It("returns a PipelineRun if the labels match with the release data", func() { + It("returns a PipelineRun if the labels match with the internal request data", func() { returnedObject, err := loader.GetInternalRequestPipelineRun(ctx, k8sClient, internalRequest) Expect(err).NotTo(HaveOccurred()) Expect(returnedObject).NotTo(Equal(&tektonv1beta1.PipelineRun{})) Expect(returnedObject.Name).To(Equal(pipelineRun.Name)) }) - It("fails to return a PipelineRun if the labels don't match with the release data", func() { + It("fails to return a PipelineRun if the labels don't match with the InternalRequest data", func() { modifiedRequest := internalRequest.DeepCopy() modifiedRequest.Name = "non-existing-request" @@ -99,6 +109,14 @@ var _ = Describe("Loader", Ordered, func() { } Expect(k8sClient.Create(ctx, internalServicesConfig)).To(Succeed()) + pipeline = &tektonv1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline", + Namespace: "default", + }, + } + Expect(k8sClient.Create(ctx, pipeline)).To(Succeed()) + pipelineRun = &tektonv1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ diff --git a/tekton/pipeline_run.go b/tekton/pipeline_run.go index e9e69a6..6358c8d 100644 --- a/tekton/pipeline_run.go +++ b/tekton/pipeline_run.go @@ -18,8 +18,11 @@ package tekton import ( "fmt" + libhandler "github.com/operator-framework/operator-lib/handler" "github.com/redhat-appstudio/internal-services/api/v1alpha1" tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "reflect" "strings" @@ -38,38 +41,35 @@ var ( InternalRequestNamespaceLabel = fmt.Sprintf("%s/%s", internalRequestLabelPrefix, "namespace") ) -// NewPipelineRun creates a PipelineRun for the given InternalRequest. The name will be autogenerated, +// InternalRequestPipelineRun is a PipelineRun alias, so we can add new methods to it in this file. +type InternalRequestPipelineRun struct { + tektonv1beta1.PipelineRun +} + +// NewInternalRequestPipelineRun creates a PipelineRun for the given InternalRequest. The name will be autogenerated, // using the name of the request as the prefix. The Pipeline information and namespace for the PipelineRun will be // extracted from the given InternalServicesConfig. -func NewPipelineRun(internalRequest *v1alpha1.InternalRequest, internalServicesConfig *v1alpha1.InternalServicesConfig) *tektonv1beta1.PipelineRun { - namespace := internalServicesConfig.Spec.Catalog.Namespace - if namespace == "" { - namespace = internalServicesConfig.Namespace - } - - pipelineRun := &tektonv1beta1.PipelineRun{ +func NewInternalRequestPipelineRun(internalServicesConfig *v1alpha1.InternalServicesConfig) *InternalRequestPipelineRun { + pipelineRun := tektonv1beta1.PipelineRun{ ObjectMeta: v1.ObjectMeta{ GenerateName: strings.ToLower(reflect.TypeOf(v1alpha1.InternalRequest{}).Name()) + "-", - Namespace: namespace, - Labels: map[string]string{ - InternalRequestNameLabel: internalRequest.Name, - InternalRequestNamespaceLabel: internalRequest.Namespace, - }, - }, - Spec: tektonv1beta1.PipelineRunSpec{ - PipelineRef: getPipelineRef(internalRequest, internalServicesConfig), + Namespace: internalServicesConfig.Namespace, }, } - appendInternalRequestParams(pipelineRun, internalRequest) + return &InternalRequestPipelineRun{pipelineRun} +} - return pipelineRun +// AsPipelineRun casts the InternalRequestPipelineRun to PipelineRun, so it can be used in the Kubernetes client. +func (i *InternalRequestPipelineRun) AsPipelineRun() *tektonv1beta1.PipelineRun { + return &i.PipelineRun } -// appendInternalRequestParams appends the InternalRequest parameters to the PipelineRun passed as an argument. -func appendInternalRequestParams(pipelineRun *tektonv1beta1.PipelineRun, internalRequest *v1alpha1.InternalRequest) { +// WithInternalRequest appends the InternalRequest parameters to the PipelineRun passed as an argument and set labels +// referencing the InternalRequest. +func (i *InternalRequestPipelineRun) WithInternalRequest(internalRequest *v1alpha1.InternalRequest) *InternalRequestPipelineRun { for param, value := range internalRequest.Spec.Params { - pipelineRun.Spec.Params = append(pipelineRun.Spec.Params, tektonv1beta1.Param{ + i.Spec.Params = append(i.Spec.Params, tektonv1beta1.Param{ Name: param, Value: tektonv1beta1.ArrayOrString{ Type: tektonv1beta1.ParamTypeString, @@ -77,46 +77,49 @@ func appendInternalRequestParams(pipelineRun *tektonv1beta1.PipelineRun, interna }, }) } -} -// getPipelineRef returns a PipelineRef generated from the information specified in the given InternalRequest and InternalServicesConfig. -func getPipelineRef(internalRequest *v1alpha1.InternalRequest, internalServicesConfig *v1alpha1.InternalServicesConfig) *tektonv1beta1.PipelineRef { - if internalServicesConfig.Spec.Catalog.Bundle == "" { - return &tektonv1beta1.PipelineRef{ - Name: internalRequest.Spec.Request, - } - } - return &tektonv1beta1.PipelineRef{ - ResolverRef: getBundleResolver(internalServicesConfig.Spec.Catalog.Bundle, internalRequest.Spec.Request), + i.ObjectMeta.Labels = map[string]string{ + InternalRequestNameLabel: internalRequest.Name, + InternalRequestNamespaceLabel: internalRequest.Namespace, } + + return i } -// getBundleResolver returns a bundle ResolverRef for the given bundle and pipeline. -func getBundleResolver(bundle, pipeline string) tektonv1beta1.ResolverRef { - return tektonv1beta1.ResolverRef{ - Resolver: "bundles", - Params: []tektonv1beta1.Param{ - { - Name: "bundle", - Value: tektonv1beta1.ParamValue{ - Type: tektonv1beta1.ParamTypeString, - StringVal: bundle, - }, - }, - { - Name: "kind", - Value: tektonv1beta1.ParamValue{ - Type: tektonv1beta1.ParamTypeString, - StringVal: "pipeline", - }, - }, - { - Name: "name", - Value: tektonv1beta1.ParamValue{ - Type: tektonv1beta1.ParamTypeString, - StringVal: pipeline, +// WithOwner set's owner annotations to the InternalRequest PipelineRun. +func (i *InternalRequestPipelineRun) WithOwner(internalRequest *v1alpha1.InternalRequest) *InternalRequestPipelineRun { + _ = libhandler.SetOwnerAnnotations(internalRequest, i) + + return i +} + +// WithPipeline sets a PipelineRef to point to the specified pipeline. It will also add a Workspace if the Pipeline +// requires one with a name matching the name of the VolumeClaim defined in the InternalServicesConfig. +func (i *InternalRequestPipelineRun) WithPipeline(pipeline *tektonv1beta1.Pipeline, internalServicesConfig *v1alpha1.InternalServicesConfig) *InternalRequestPipelineRun { + i.Spec.PipelineRef = &tektonv1beta1.PipelineRef{ + Name: pipeline.Name, + } + + for _, workspace := range pipeline.Spec.Workspaces { + if workspace.Name == internalServicesConfig.Spec.VolumeClaim.Name { + i.Spec.Workspaces = []tektonv1beta1.WorkspaceBinding{ + { + Name: internalServicesConfig.Spec.VolumeClaim.Name, + VolumeClaimTemplate: &corev1.PersistentVolumeClaim{ + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse(internalServicesConfig.Spec.VolumeClaim.Size), + }, + }, + }, + }, }, - }, - }, + } + break + } } + + return i } diff --git a/tekton/pipeline_run_test.go b/tekton/pipeline_run_test.go index 622487e..2e4c750 100644 --- a/tekton/pipeline_run_test.go +++ b/tekton/pipeline_run_test.go @@ -5,7 +5,7 @@ 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 + 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, @@ -13,12 +13,12 @@ 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 tekton import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/operator-framework/operator-lib/handler" "github.com/redhat-appstudio/internal-services/api/v1alpha1" tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,135 +32,133 @@ var _ = Describe("PipelineRun", Ordered, func() { internalServicesConfig *v1alpha1.InternalServicesConfig internalRequest *v1alpha1.InternalRequest - pipelineRun *tektonv1beta1.PipelineRun + pipeline *tektonv1beta1.Pipeline ) BeforeAll(func() { createResources() }) - Context("When calling NewPipelineRun", func() { + Context("When calling NewInternalRequestPipelineRun", func() { It("should return a PipelineRun named after the InternalRequest", func() { - newPipelineRun := NewPipelineRun(internalRequest, internalServicesConfig) + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) generatedName := strings.ToLower(reflect.TypeOf(v1alpha1.InternalRequest{}).Name()) + "-" - Expect(newPipelineRun.GenerateName).To(ContainSubstring(generatedName)) - }) - - It("should contain the InternalRequest labels", func() { - newPipelineRun := NewPipelineRun(internalRequest, internalServicesConfig) - - Expect(newPipelineRun.Labels[InternalRequestNameLabel]).To(Equal(internalRequest.Name)) - Expect(newPipelineRun.Labels[InternalRequestNamespaceLabel]).To(Equal(internalRequest.Namespace)) + Expect(newInternalRequestPipelineRun.GenerateName).To(ContainSubstring(generatedName)) }) - It("should target the InternalServicesConfig namespace if spec.catalog.namespace is not set", func() { + It("should target the InternalServicesConfig namespace", func() { newInternalServicesConfig := &v1alpha1.InternalServicesConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "internalServicesConfig", Namespace: "internalServicesConfig-namespace", }, } - newPipelineRun := NewPipelineRun(internalRequest, newInternalServicesConfig) + newPipelineRun := NewInternalRequestPipelineRun(newInternalServicesConfig) Expect(newPipelineRun.Namespace).To(Equal("internalServicesConfig-namespace")) }) + }) - It("should target the InternalServicesConfig spec.catalog.namespace if set", func() { - newPipelineRun := NewPipelineRun(internalRequest, internalServicesConfig) + Context("When calling AsPipelineRun", func() { + It("should return a PipelineRun representing the InternalRequest PipelineRun", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + pipelineRun := newInternalRequestPipelineRun.AsPipelineRun() - Expect(newPipelineRun.Namespace).To(Equal(internalServicesConfig.Namespace)) + Expect(reflect.TypeOf(pipelineRun)).To(Equal(reflect.TypeOf(&tektonv1beta1.PipelineRun{}))) + generatedName := strings.ToLower(reflect.TypeOf(v1alpha1.InternalRequest{}).Name()) + "-" + Expect(pipelineRun.GenerateName).To(ContainSubstring(generatedName)) + Expect(pipelineRun.Namespace).To(Equal(internalServicesConfig.Namespace)) }) + }) - It("should reference the Pipeline specified in the InternalRequest", func() { - newPipelineRun := NewPipelineRun(internalRequest, internalServicesConfig) + Context("When calling WithInternalRequest", func() { + It("should append to the PipelineRun the parameters specified in the InternalRequest", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithInternalRequest(internalRequest) - Expect(newPipelineRun.Spec.PipelineRef.Name).To(Equal(internalRequest.Spec.Request)) + Expect(newInternalRequestPipelineRun.Spec.Params).To(HaveLen(len(internalRequest.Spec.Params))) + for _, param := range newInternalRequestPipelineRun.Spec.Params { + Expect(param.Value.StringVal).To(Equal(internalRequest.Spec.Params[param.Name])) + } }) - It("should reference a bundle if set in the InternalServicesConfig", func() { - newInternalServicesConfig := &v1alpha1.InternalServicesConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "internalServicesConfig", - Namespace: "internalServicesConfig-namespace", - }, - Spec: v1alpha1.InternalServicesConfigSpec{ - Catalog: v1alpha1.Catalog{ - Bundle: "quay.io/foo/bar:baz", - }, - }, - } - newPipelineRun := NewPipelineRun(internalRequest, newInternalServicesConfig) - - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef).NotTo(Equal(tektonv1beta1.ResolverRef{})) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Resolver).To(Equal(tektonv1beta1.ResolverName("bundles"))) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params).To(HaveLen(3)) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[0].Name).To(Equal("bundle")) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[0].Value.StringVal).To(Equal(newInternalServicesConfig.Spec.Catalog.Bundle)) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[1].Name).To(Equal("kind")) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[1].Value.StringVal).To(Equal("pipeline")) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[2].Name).To(Equal("name")) - Expect(newPipelineRun.Spec.PipelineRef.ResolverRef.Params[2].Value.StringVal).To(Equal(internalRequest.Spec.Request)) + It("should contain the InternalRequest labels", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithInternalRequest(internalRequest) + + Expect(newInternalRequestPipelineRun.Labels[InternalRequestNameLabel]).To(Equal(internalRequest.Name)) + Expect(newInternalRequestPipelineRun.Labels[InternalRequestNamespaceLabel]).To(Equal(internalRequest.Namespace)) }) }) - Context("When calling appendInternalRequestParams", func() { - It("should append to the PipelineRun the parameters specified in the InternalRequest", func() { - newPipelineRun := pipelineRun.DeepCopy() - appendInternalRequestParams(newPipelineRun, internalRequest) + Context("When calling WithOwner", func() { + It("should add ownership annotations", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithOwner(internalRequest) - Expect(newPipelineRun.Spec.Params).To(HaveLen(len(internalRequest.Spec.Params))) - for _, param := range newPipelineRun.Spec.Params { - Expect(param.Value.StringVal).To(Equal(internalRequest.Spec.Params[param.Name])) - } + Expect(newInternalRequestPipelineRun.Annotations).To(HaveLen(2)) + Expect(newInternalRequestPipelineRun.Annotations[handler.NamespacedNameAnnotation]).To( + Equal(internalRequest.Namespace + "/" + internalRequest.Name), + ) + Expect(newInternalRequestPipelineRun.Annotations[handler.TypeAnnotation]).To(Equal(internalRequest.Kind)) }) }) - Context("When calling getPipelineRef", func() { - It("should return a PipelineRef without resolver if the internalServicesConfig contains no bundle", func() { - pipelineRef := getPipelineRef(internalRequest, internalServicesConfig) - Expect(pipelineRef.Name).To(Equal(internalRequest.Name)) - Expect(pipelineRef.ResolverRef).To(Equal(tektonv1beta1.ResolverRef{})) + Context("When calling WithPipeline", func() { + It("should reference the Pipeline passed as an argument", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithPipeline(pipeline, internalServicesConfig) + + Expect(newInternalRequestPipelineRun.Spec.PipelineRef).NotTo(BeNil()) + Expect(newInternalRequestPipelineRun.Spec.PipelineRef.Name).To(Equal(pipeline.Name)) }) - It("should return a PipelineRef with a bundle resolver if the internalServicesConfig contains a bundle", func() { - newInternalServicesConfig := &v1alpha1.InternalServicesConfig{ + It("should not contain a workspace if the Pipeline doesn't specify one", func() { + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithPipeline(pipeline, internalServicesConfig) + + Expect(newInternalRequestPipelineRun.Spec.Workspaces).To(HaveLen(0)) + }) + + It("should not contain a workspace if the Pipeline specify one with a different name from the one in the InternalServicesConfig", func() { + newPipeline := &tektonv1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ - Name: "internalServicesConfig", - Namespace: "internalServicesConfig-namespace", + Name: "pipeline", + Namespace: "default", }, - Spec: v1alpha1.InternalServicesConfigSpec{ - Catalog: v1alpha1.Catalog{ - Bundle: "quay.io/foo/bar:baz", + Spec: tektonv1beta1.PipelineSpec{ + Workspaces: []tektonv1beta1.PipelineWorkspaceDeclaration{ + {Name: "foo"}, }, }, } - pipelineRef := getPipelineRef(internalRequest, newInternalServicesConfig) - Expect(pipelineRef.Name).To(BeEmpty()) - Expect(pipelineRef.ResolverRef).NotTo(Equal(tektonv1beta1.ResolverRef{})) - Expect(pipelineRef.ResolverRef.Resolver).To(Equal(tektonv1beta1.ResolverName("bundles"))) - Expect(pipelineRef.ResolverRef.Params).To(HaveLen(3)) - Expect(pipelineRef.ResolverRef.Params[0].Name).To(Equal("bundle")) - Expect(pipelineRef.ResolverRef.Params[0].Value.StringVal).To(Equal(newInternalServicesConfig.Spec.Catalog.Bundle)) - Expect(pipelineRef.ResolverRef.Params[1].Name).To(Equal("kind")) - Expect(pipelineRef.ResolverRef.Params[1].Value.StringVal).To(Equal("pipeline")) - Expect(pipelineRef.ResolverRef.Params[2].Name).To(Equal("name")) - Expect(pipelineRef.ResolverRef.Params[2].Value.StringVal).To(Equal(internalRequest.Spec.Request)) + + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithPipeline(newPipeline, internalServicesConfig) + + Expect(newInternalRequestPipelineRun.Spec.Workspaces).To(HaveLen(0)) }) - }) - Context("When calling getBundleResolver", func() { - It("should return a bundle resolver referencing the InternalServicesConfig bundle and Pipeline", func() { - resolver := getBundleResolver("quay.io/foo/bar:baz", "pipeline") - Expect(resolver).NotTo(Equal(tektonv1beta1.ResolverRef{})) - Expect(resolver.Resolver).To(Equal(tektonv1beta1.ResolverName("bundles"))) - Expect(resolver.Params).To(HaveLen(3)) - Expect(resolver.Params[0].Name).To(Equal("bundle")) - Expect(resolver.Params[0].Value.StringVal).To(Equal("quay.io/foo/bar:baz")) - Expect(resolver.Params[1].Name).To(Equal("kind")) - Expect(resolver.Params[1].Value.StringVal).To(Equal("pipeline")) - Expect(resolver.Params[2].Name).To(Equal("name")) - Expect(resolver.Params[2].Value.StringVal).To(Equal("pipeline")) + It("should contain a workspace if the Pipeline specify one with the same name seen in the InternalServicesConfig", func() { + newPipeline := &tektonv1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline", + Namespace: "default", + }, + Spec: tektonv1beta1.PipelineSpec{ + Workspaces: []tektonv1beta1.PipelineWorkspaceDeclaration{ + {Name: internalServicesConfig.Spec.VolumeClaim.Name}, + }, + }, + } + + newInternalRequestPipelineRun := NewInternalRequestPipelineRun(internalServicesConfig) + newInternalRequestPipelineRun.WithPipeline(newPipeline, internalServicesConfig) + + Expect(newInternalRequestPipelineRun.Spec.Workspaces).To(HaveLen(1)) + Expect(newInternalRequestPipelineRun.Spec.Workspaces[0].Name).To(Equal(internalServicesConfig.Spec.VolumeClaim.Name)) + Expect(newInternalRequestPipelineRun.Spec.Workspaces[0].VolumeClaimTemplate).NotTo(BeNil()) }) }) @@ -178,17 +176,24 @@ var _ = Describe("PipelineRun", Ordered, func() { }, }, } + internalRequest.Kind = "InternalRequest" internalServicesConfig = &v1alpha1.InternalServicesConfig{ ObjectMeta: metav1.ObjectMeta{ Name: v1alpha1.InternalServicesConfigResourceName, Namespace: "default", }, + Spec: v1alpha1.InternalServicesConfigSpec{ + VolumeClaim: v1alpha1.VolumeClaim{ + Name: "workspace", + Size: "1Gi", + }, + }, } - pipelineRun = &tektonv1beta1.PipelineRun{ + pipeline = &tektonv1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ - Name: "pipeline-run", + Name: "pipeline", Namespace: "default", }, }