Pick up build strategy knowledge from BuildStrategy API about build

[siamaksade]

This was discussed for Tekton as well but community opted for Task/ClusterTask, TriggerBinding/ClusterTriggerBidning, etc to avoid the RBAC issues when referencing another namespace. The other argument was that this pattern of referencing a resource in a different namespace generally does not exist on Kubernetes and there too are separate objects for Role/ClusterRole, RoleBinding/ClusterRoleBinding.

The other issue with referencing a namespace is what we have with imagestreams and templates: one would not know what is available on the cluster unless they are aware of the special treatment the 'openshift' namespace gets.

[siamaksade]

That’s a valid point too. Shall we just have a cluster-scope strategy for now and gauge the interest for a namespace-scoped one?

[zhangtbj]

Sure thing @sbose78 ,

I will prepare the PR which follow by the above flow this week.

And will let you know or discuss the PR later together.

[sbose78]

Making use of https://github.com/otaviof/tekton-pipeline-chart/blob/672961906029b9c5a4c6a0f0051c3655bed64dc9/templates/10-task.yaml & https://github.com/otaviof/tekton-pipeline-chart/blob/672961906029b9c5a4c6a0f0051c3655bed64dc9/templates/_steps-buildpacks.tpl & https://github.com/otaviof/tekton-pipeline-chart/blob/672961906029b9c5a4c6a0f0051c3655bed64dc9/values-custom.yaml

I could come up with this:

apiVersion: build.dev/v1alpha1
kind: BuildStrategy
metadata:
  name: custom-buildpacks-v3-strategy
spec:
  source:
    name: repository
    type: git
  steps:
    - name: prepare
      image: alpine
      securityContext:
        runAsUser: 0
      command:
        - /bin/sh
      args:
        - -c
        - chown -R "1000:1000" "/workspace/{{ source.name }}" #from the same YAML
    - name: detect
      image: {{ build.builderImage }} #from Build CR
      securityContext:
        runAsUser: 1000
      command:
        - /cnb/lifecycle/detector
      args:
        - -log-level=debug
        - -app=/workspace/{{ source.name }} #from the same YAML
        - -group=/layers/group.toml
        - -plan=/layers/plan.toml
      volumeMounts:
        - name: layers-dir
          mountPath: /layers
    - name: analyze
      image: {{ build.builderImage }} #from Build CR
      securityContext:
        runAsUser: 1000
      command:
        - /cnb/lifecycle/analyzer
      args:
        - -log-level=debug
        - -layers=/layers
        - -group=/layers/group.toml
        - {{ build.outputImage }} #from Build CR
      volumeMounts:
        - name: layers-dir
          mountPath: /layers
    - name: restore
      image: {{ build.outputImage }} #from Build CR
      securityContext:
        runAsUser: 1000
      command:
        - /cnb/lifecycle/restorer
      args:
        - -log-level=debug
        - -group=/layers/group.toml
        - -layers=/layers
        - -path=/cache
      volumeMounts:
        - name: cache-dir
          mountPath: /cache
        - name: layers-dir
          mountPath: /layers
    - name: build
      image: {{ build.outputImage }} #from Build CR
      securityContext:
        runAsUser: 1000
      command:
        - /cnb/lifecycle/builder
      args:
        - -app=/workspace/{{ source.name }}
        - -layers=/layers
        - -group=/layers/group.toml
        - -plan=/layers/plan.toml
      volumeMounts:
        - name: layers-dir
          mountPath: /layers

[sbose78]

And similarly,

apiVersion: build.dev/v1alpha1
kind: BuildStrategy
metadata:
  name: custom-s2i-strategy
spec:
  source:
    name: repository ## eventually gets mapped into a Task.Spec.Resources[0]
    type: git
  steps:
    - name: s2i-build-as-dockerfile
      image: otaviof/s2i:latest
      workingDir: /workspace/{{ source.name }}  #from the same YAML
      args:
        - build
        - .
        - {{ build.builderImage }} #from Build CR
        - otaviof/nodejs-ex:latest
        - --as-dockerfile=Dockerfile
    - name: buildah-bud
      image: quay.io/buildah/stable:latest
      workingDir: /workspace/{{ source.name }}  #from the same YAML
      args:
        - buildah
        - bud
        - --tag="otaviof/nodejs-ex:latest"
        - /workspace/{{ source.name }}  #from the same YAML

[siamaksade]

Do we need BuildStrategy and ClusterBuildStrategy to allow both cluster-scope and namespace-scoped strategies?

[sbose78]

I'm avoiding ClusterBuildStrategy to begin with.

In the current state of the API,
you can specify the namespace, optionally. if not specified, we look for it in the same namespace. but we could change what the default behaviour is.

  # Strategy defined in the buildpacks-v3 CR 
  # in the 'openshift' namespace.
  strategy: 
    name: "buildpacks-v3"
    namespace: "openshift"

---

Full CR

apiVersion: build.dev/v1alpha1
kind: Build
metadata:
  name: example-build-buildpack
spec:
  # Git Source definition
  source:
    url: https://github.com/sclorg/nodejs-ex
    credentials:
      name: github-auth-sbose78

  # Strategy defined in the buildpacks-v3 CR 
  # in the 'openshift' namespace.
  strategy: 
    name: "buildpacks-v3"
    namespace: "openshift"

  # Build to be run in this image.
  builderImage: "cloudfoundry/cnb:bionic"

  # Generated image.
  output:
    image: "image-registry.openshift-image-registry.svc:5000/sbose/nodejs-ex"
    credentials:
      name: github-auth-sbose78

[sbose78]

This was discussed for Tekton as well but community opted for Task/ClusterTask, TriggerBinding/ClusterTriggerBidning, etc to avoid the RBAC issues when referencing another namespace. The other argument was that this pattern of referencing a resource in a different namespace generally does not exist on Kubernetes and there too are separate objects for Role/ClusterRole, RoleBinding/ClusterRoleBinding.

I was trying to keep the number of APIs/CRDs low, without a very strong opinion.
However, if you have your weight behind this, I'm Okay with have with a ClusterBuildStrategy 👍

The other issue with referencing a namespace is what we have with imagestreams and templates: one would not know what is available on the cluster unless they are aware of the special treatment the 'openshift' namespace gets.

Agreed, and vanilla Kubernetes will not even have such a namespace.

[sbose78]

Yup, we could do that.

[zhangtbj]

Hi @siamaksade or @sbose78 ,

We also require this ClusterBuildStrategy feature. And I think there is a valid case :)

I saw your build spec definition is:

  // StrategyRef refers to the BuildStrategy to be used to
  // build the container image.
  // Note: Using metav1.ObjectMeta  instead of corev1.LocalObjectReference
  // because the BuildStrategy may or may not be in the same namespace.
  StrategyRef metav1.ObjectMeta `json:"strategy"`

Our platform is a multi-tenants platform.

• It means we cannot create all same buildstrategies in each tenant namespaces.
• And it is also not good that a build in a namespace calls the buildstrategy in another namespace.

I also suggest that do the samiliar way like Tekton (Such as Task and ClusterTask) that:

1. Provide a BuildStrategy and ClusterBuildStrategy
2. Refine the StrategyRef from metav1.ObjectMeta to a new StrategyRef type like :

type StrategyRef struct {
	// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
	Name string `json:"name,omitempty"`
	// StrategyKind inficates the kind of the buildstrategy, namespaced or cluster scoped.
	Kind StrategyKind `json:"kind,omitempty"`
	// API version of the referent
	// +optional
	APIVersion string `json:"apiVersion,omitempty"`
}

3. Then provide the addtional logic in build controller retrieveCustomBuildStrategy method to get differnt types of the buildstrategy:

if tr.Spec.StrategyRef != nil && tr.Spec.StrategyRef.Kind == v1alpha1.ClusterBuildStrategyKind {
  ...
  buildStrategyInstance = fake_get_from_cluster_scope()
} else {
  ...
  buildStrategyInstance = fake_get_from_namespaced_scope()
}

I am not sure did you start developing this feature?

Because this is a blocker of our design, if doesn't start, we can pick up and do it soon.

Pls let me know if you have any suggestion or comment :)

[sbose78]

Hey! Thank you!
I haven't started working on it, please go ahead. I would suggest you have a PR open early so that we can collaborate?

[zhangtbj]

Hi @sbose78 ,

I provided a PR (#62) for this new ClusterBuildStrategy support. The introduction is in the PR comment. Please review and let me know if you have any comment or idea, or we have have a quick call or meeting to review or discuss the PR.

BTW, during the development, I also see there are some improvements that we can do, such as:

• Add API validation
• Add API default setting (Such as set default value for the optional parameters)
• Add operator-sdk fake client for deep test coverage in controller unit test (https://github.com/operator-framework/operator-sdk/blob/master/doc/user/unit-testing.md)
• If it is possible to generate the TaskRun name instead of using the hardcode name from build name, so that can store more build log?
• There is a problem when executing function applyCredentials, if there is a wrong or bad secret in the serviceaccount, this func won't update the secrets in sa, and always report error and end-user is hard to know the real problem

Now we are verifying running build v2 on our platform. If possible, we can also have some discussions, and we can help improve them one by one.