feat(RELEASE-1327): add pipeline field to InternalRequests

This commit adds a Pipeline field to the InternalRequest Spec. This
allows passing a pipeline using any kind of resolver, whereas the
existing request field only allows for a cluster pipeline to be used.

It also updates the manager to use this pipeline field and adds helper
functions.

Signed-off-by: Johnny Bieren <[email protected]>

api/v1alpha1/internalrequest_types.go

@@ -22,6 +22,7 @@ import (
 	"github.com/konflux-ci/operator-toolkit/conditions"
 
 	"github.com/konflux-ci/internal-services/metrics"
+	tektonutils "github.com/konflux-ci/internal-services/tekton/utils"
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,8 +32,12 @@ import (
 type InternalRequestSpec struct {
 	// Request is the name of the internal internalrequest which will be translated into a Tekton pipeline
 	// +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
-	// +required
-	Request string `json:"request"`
+	// +optional
+	Request string `json:"request,omitempty"`
+
+	// Pipeline contains the details of the pipeline to execute for the InternalRequest
+	// +optional
+	Pipeline *tektonutils.ParameterizedPipeline `json:"pipeline,omitempty"`
 
 	// Params is the list of optional parameters to pass to the Tekton pipeline
 	// kubebuilder:pruning:PreserveUnknownFields

config/crd/bases/appstudio.redhat.com_internalrequests.yaml

@@ -48,6 +48,81 @@ spec:
                 description: Params is the list of optional parameters to pass to
                   the Tekton pipeline kubebuilder:pruning:PreserveUnknownFields
                 type: object
+              pipeline:
+                description: Pipeline contains the details of the pipeline to execute
+                  for the InternalRequest
+                properties:
+                  params:
+                    description: Params is a slice of parameters for a given resolver
+                    items:
+                      description: Param defines the parameters for a given resolver
+                        in PipelineRef
+                      properties:
+                        name:
+                          description: Name is the name of the parameter
+                          type: string
+                        value:
+                          description: Value is the value of the parameter
+                          type: string
+                      required:
+                      - name
+                      - value
+                      type: object
+                    type: array
+                  pipelineRef:
+                    description: PipelineRef is the reference to the Pipeline
+                    properties:
+                      params:
+                        description: Params is a slice of parameters for a given resolver
+                        items:
+                          description: Param defines the parameters for a given resolver
+                            in PipelineRef
+                          properties:
+                            name:
+                              description: Name is the name of the parameter
+                              type: string
+                            value:
+                              description: Value is the value of the parameter
+                              type: string
+                          required:
+                          - name
+                          - value
+                          type: object
+                        type: array
+                      resolver:
+                        description: Resolver is the name of a Tekton resolver to
+                          be used (e.g. git)
+                        type: string
+                    required:
+                    - params
+                    - resolver
+                    type: object
+                  serviceAccountName:
+                    description: ServiceAccountName is the ServiceAccount to use during
+                      the execution of the Pipeline
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
+                    type: string
+                  timeouts:
+                    description: Timeouts defines the different Timeouts to use in
+                      the PipelineRun execution
+                    properties:
+                      finally:
+                        description: Finally sets the maximum allowed duration of
+                          this pipeline's finally
+                        type: string
+                      pipeline:
+                        description: Pipeline sets the maximum allowed duration for
+                          execution of the entire pipeline. The sum of individual
+                          timeouts for tasks and finally must not exceed this value.
+                        type: string
+                      tasks:
+                        description: Tasks sets the maximum allowed duration of this
+                          pipeline's tasks
+                        type: string
+                    type: object
+                required:
+                - pipelineRef
+                type: object
               request:
                 description: Request is the name of the internal internalrequest which
                   will be translated into a Tekton pipeline
@@ -77,8 +152,6 @@ spec:
                       tasks
                     type: string
                 type: object
-            required:
-            - request
             type: object
           status:
             description: InternalRequestStatus defines the observed state of InternalRequest.

controllers/internalrequest/adapter.go

@@ -82,10 +82,15 @@ func (a *Adapter) EnsureConfigIsLoaded() (controller.OperationResult, error) {
 
 // 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.
+// marked as failed. If a resolver is being used, this operation does nothing. We will use resolvers only going forward,
+// so this operation will be removed in a future commit.
 //
 // Note: This operation sets values in the adapter to be used by other operations, so it should be always enabled.
 func (a *Adapter) EnsurePipelineExists() (controller.OperationResult, error) {
+	if a.internalRequest.Spec.Request == "" {
+		return controller.ContinueProcessing()
+	}
+
 	var err error
 	a.internalRequestPipeline, err = a.loader.GetInternalRequestPipeline(a.ctx, a.internalClient,
 		a.internalRequest.Spec.Request, a.internalServicesConfig.Namespace)
@@ -190,11 +195,23 @@ func (a *Adapter) EnsureStatusIsTracked() (controller.OperationResult, error) {
 // 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()
+	var pipelineRun *v1beta1.PipelineRun
+	// Once our tasks are migrated to use Spec.Pipeline instead of Spec.Request, the first part of
+	// this if statement will go away and only pipelineRef InternalRequests will be supported.
+	// pipelineRef still allows the use of cluster resolvers, so we don't lose anything.
+	if a.internalRequestPipeline != nil {
+		pipelineRun = tekton.NewInternalRequestPipelineRun(a.internalServicesConfig).
+			WithInternalRequest(a.internalRequest).
+			WithOwner(a.internalRequest).
+			WithPipeline(a.internalRequestPipeline, a.internalServicesConfig).
+			AsPipelineRun()
+	} else {
+		pipelineRun = tekton.NewInternalRequestPipelineRun(a.internalServicesConfig).
+			WithInternalRequest(a.internalRequest).
+			WithOwner(a.internalRequest).
+			WithPipelineRef(a.internalRequest, a.internalServicesConfig).
+			AsPipelineRun()
+	}
 
 	err := a.internalClient.Create(a.ctx, pipelineRun)
 	if err != nil {

controllers/internalrequest/adapter_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/konflux-ci/internal-services/loader"
+	"github.com/konflux-ci/internal-services/tekton/utils"
 	toolkit "github.com/konflux-ci/operator-toolkit/loader"
 
 	"reflect"
@@ -93,6 +94,29 @@ var _ = Describe("PipelineRun", Ordered, func() {
 			Expect(err).To(BeNil())
 		})
 
+		It("should continue if the InternalRequest Request field is empty", func() {
+			internalRequestNoRequest := &v1alpha1.InternalRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "request",
+					Namespace: "default",
+				},
+				Spec: v1alpha1.InternalRequestSpec{
+					ServiceAccount: "sample-sa",
+				},
+			}
+			adapter.ctx = toolkit.GetMockedContext(ctx, []toolkit.MockData{
+				{
+					ContextKey: loader.InternalRequestContextKey,
+					Resource:   internalRequestNoRequest,
+				},
+			})
+			adapter.internalRequest.MarkRunning()
+
+			result, err := adapter.EnsurePipelineExists()
+			Expect(!result.CancelRequest && !result.RequeueRequest).To(BeTrue())
+			Expect(err).To(BeNil())
+		})
+
 		It("should set a 'No endpoint to handle' if the Pipeline was not found", func() {
 			adapter.ctx = toolkit.GetMockedContext(ctx, []toolkit.MockData{
 				{
@@ -324,6 +348,74 @@ var _ = Describe("PipelineRun", Ordered, func() {
 		})
 	})
 
+	Context("When calling createInternalRequestPipelineRun with a PipelineRef", func() {
+		AfterEach(func() {
+			deleteResources()
+		})
+
+		BeforeEach(func() {
+			createResources()
+			parameterizedPipeline := utils.ParameterizedPipeline{}
+			parameterizedPipeline.PipelineRef = utils.PipelineRef{
+				Resolver: "git",
+				Params: []utils.Param{
+					{Name: "url", Value: "my-url"},
+					{Name: "revision", Value: "my-revision"},
+					{Name: "pathInRepo", Value: "my-path"},
+				},
+			}
+			internalRequestPipelineRef := &v1alpha1.InternalRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "request",
+					Namespace: "default",
+				},
+				Spec: v1alpha1.InternalRequestSpec{
+					ServiceAccount: "sample-sa",
+					Pipeline:       &parameterizedPipeline,
+				},
+			}
+			// Set a proper Kind
+			internalRequestPipelineRef.TypeMeta = metav1.TypeMeta{
+				Kind: "InternalRequest",
+			}
+			adapter.internalRequest = internalRequestPipelineRef
+			adapter.internalRequest.MarkRunning()
+		})
+
+		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(3))
+			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.ResolverRef.Params).Should(ContainElement(HaveField("Value.StringVal", "my-url")))
+		})
+
+		It("creates a PipelineRun with the proper service account", func() {
+			pipelineRun, err := adapter.createInternalRequestPipelineRun()
+			Expect(pipelineRun).NotTo(BeNil())
+			Expect(err).To(BeNil())
+			Expect(pipelineRun.Spec.ServiceAccountName).To(Equal("sample-sa"))
+		})
+	})
+
 	Context("When calling getDefaultInternalServicesConfig", func() {
 		AfterEach(func() {
 			deleteResources()

tekton/pipeline_run.go

@@ -135,3 +135,45 @@ func (i *InternalRequestPipelineRun) WithPipeline(pipeline *tektonv1beta1.Pipeli
 
 	return i
 }
+
+// WithPipelineRef sets a PipelineRef to point to the specified pipeline. It will also add a Workspace to the Pipeline
+// with a name matching the name of the VolumeClaim defined in the InternalServicesConfig.
+func (i *InternalRequestPipelineRun) WithPipelineRef(internalRequest *v1alpha1.InternalRequest, internalServicesConfig *v1alpha1.InternalServicesConfig) *InternalRequestPipelineRun {
+	i.Spec.PipelineRef = internalRequest.Spec.Pipeline.PipelineRef.ToTektonPipelineRef()
+	if i.Spec.PipelineRef.Resolver == "cluster" {
+		i.Spec.PipelineRef.Params = append(i.Spec.PipelineRef.Params,
+			tektonv1beta1.Param{
+				Name: "namespace",
+				Value: tektonv1beta1.ParamValue{
+					Type:      tektonv1beta1.ParamTypeString,
+					StringVal: internalRequest.Namespace,
+				},
+			},
+			tektonv1beta1.Param{
+				Name: "kind",
+				Value: tektonv1beta1.ParamValue{
+					Type:      tektonv1beta1.ParamTypeString,
+					StringVal: "pipeline",
+				},
+			},
+		)
+	}
+
+	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),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	return i
+}