Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New build task : paketo builder #1660

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cmoulliard
Copy link

@cmoulliard cmoulliard commented Nov 28, 2024

  • New build task: paketo builder
  • The build-paketo-ubi-builder task builds the ubi builder image for paketo using as input the builder.toml file. The image is build using the pack tool packaged part of the paketo-container image.
    The task also produces the SBOM which is signed and added to the image.

Build succeeded part of this PR: https://console.redhat.com/application-pipeline/workspaces/cmoullia/applications/buildpack-remote/pipelineruns/builder-ubi-base-l2q8r

Screenshot 2024-11-28 at 15 46 54

@mmorhun
Copy link
Collaborator

mmorhun commented Nov 29, 2024

@cmoulliard what are the obstacles to use common buildah(-remote) task?

@cmoulliard
Copy link
Author

what are the obstacles to use common buildah(-remote) task?

What do you mean by "to use common buildah task" ? If the question is about: "Can we build the ubi builder image using buildah then the answer is no as such an image is not build from a Dockerfile by using a tool: pack - https://github.com/buildpacks/pack"

@mmorhun
Copy link
Collaborator

mmorhun commented Nov 29, 2024

@cmoulliard have you considered running pack inside a build stage and just copying the content to the final stage?

- name: BUILDER_IMAGE
description: The image packaging the paketo tools and to be used to build
type: string
default: "quay.io/redhat-user-workloads/konflux-build-pipeli-tenant/paketo-container:ea8ddb8818bb4a55546927e7674b0362dabd6342"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we cannot allow builder image to be specified as param, it must be hardcoded in task to provide valid provenance, other build tasks have been updated to disallow such parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also image should be officially released in konflux-ci quay namespace, we don't allow images from user-workloads. This image should be properly released via releases to konflux-ci

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also image should be officially released in konflux-ci quay namespace, we don't allow images from user-workloads. This image should be properly released via releases to konflux-ci

What should we do to release it part of konflux-ci ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the default value in the meantime. See: a328f42

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have to configure your konflux instance with proper release plan

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot allow builder image to be specified as param

and what we do about this part?

Copy link
Author

@cmoulliard cmoulliard Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and what we do about this part?

What do you mean ? @MartinBasti

Copy link
Contributor

@MartinBasti MartinBasti Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot allow users to specify their own build images, it must be hardcoded in the task to provide correct provenance

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So asap as this image will be released by konflux, then we will set the image ref to be used.

@cmoulliard
Copy link
Author

have you considered running pack inside a build stage and just copying the content to the final stage?

What is a build stage ? Can you elaborate ?

@mmorhun
Copy link
Collaborator

mmorhun commented Dec 2, 2024

What is a build stage ? Can you elaborate ?

Sorry for not being clear enough, I meant this

@MartinBasti
Copy link
Contributor

/verify-owners

@cmoulliard
Copy link
Author

Sorry for not being clear enough, I meant this

Thanks. We cannot use Dockerfile like Multi-stage to build the buildpack stuffs (builder image or stack image or buildpacks) but specific tools like: pack, jam, create-package, etc

@MartinBasti
Copy link
Contributor

/verify-owners

@arewm
Copy link
Member

arewm commented Dec 4, 2024

/retest

@cmoulliard
Copy link
Author

Can we merge it ?

@MartinBasti
Copy link
Contributor

Can we merge it ?

No, it has no approvals and unresolved threads

@chmeliik
Copy link
Contributor

chmeliik commented Dec 6, 2024

We have changed the ownership mechanism from OWNERS to CODEOWNERS. Please rebased on main and update CODEOWNERS instead of OWNERS 🙏

@cmoulliard cmoulliard requested a review from a team as a code owner December 9, 2024 08:39
@cmoulliard
Copy link
Author

We have changed the ownership mechanism from OWNERS to CODEOWNERS. Please rebased on main and update CODEOWNERS instead of OWNERS 🙏

Done: See: 9d34267 and 369a77c

CODEOWNERS Show resolved Hide resolved
@rcerven rcerven removed their request for review December 9, 2024 13:58
@MartinBasti
Copy link
Contributor

description: the arguments to be passed to the pack command to build the image
type: array
default: []
- name: BUILDER_IMAGE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builder image as param is still here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with: e475d5e1 @MartinBasti

@MartinBasti
Copy link
Contributor

Please squash commits into one with nice description

Review the wording of the intro text

Signed-off-by: cmoulliard <[email protected]>

Changing the OWNERS

Signed-off-by: cmoulliard <[email protected]>

Use kerberios username for cmoulliard

Signed-off-by: cmoulliard <[email protected]>

Fix username typo error

Signed-off-by: cmoulliard <[email protected]>

Remove ubi from name, README and task definition

Signed-off-by: cmoulliard <[email protected]>

Removing the default value image as name will change

Signed-off-by: cmoulliard <[email protected]>

Removing #rsync comments

Signed-off-by: cmoulliard <[email protected]>

Remove from message printed:

Signed-off-by: cmoulliard <[email protected]>

Use buildah --retry parameter

Signed-off-by: cmoulliard <[email protected]>

Rename the task to include the suffic: -oci-ta

Signed-off-by: cmoulliard <[email protected]>

Change owner from cmoullia to cmoulliard

Signed-off-by: cmoulliard <[email protected]>

Double quote to prevent globbing and word splitting

Signed-off-by: cmoulliard <[email protected]>

Double quote to prevent globbing and word splitting

Signed-off-by: cmoulliard <[email protected]>

Double quote to prevent word splitting

Signed-off-by: cmoulliard <[email protected]>

Declare and assign separately to avoid masking return values.

Signed-off-by: cmoulliard <[email protected]>

Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.

Signed-off-by: cmoulliard <[email protected]>

Remove trailing spaces and wrong indentation: expected 4 but found 6

Signed-off-by: cmoulliard <[email protected]>

Fixing: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate

Signed-off-by: cmoulliard <[email protected]>

Fixing: Expanding an array without an index only gives the first element.

Signed-off-by: cmoulliard <[email protected]>

Fixing: Use braces when expanding arrays.

Signed-off-by: cmoulliard <[email protected]>

Remove double quote for ${BUILD_ARGS[@]}

Signed-off-by: cmoulliard <[email protected]>

Rename the task to include the suffix: -oci-ta

Signed-off-by: cmoulliard <[email protected]>

Replace @ with * for the array

Signed-off-by: cmoulliard <[email protected]>

Replace @ with * for the array

Signed-off-by: cmoulliard <[email protected]>

Removing some additional comments

Signed-off-by: cmoulliard <[email protected]>

Moving from OWNERS to CODEOWNERS

Signed-off-by: cmoulliard <[email protected]>

Add missing task of paketo to the renovate.json file

Signed-off-by: cmoulliard <[email protected]>

Updating renovate.json using update_renovate_json_based_on_codeowners.py script

Signed-off-by: cmoulliard <[email protected]>

Create a new renovate group for paketo => buildpack

Signed-off-by: cmoulliard <[email protected]>

Set -x to debug the bash script and review the logic to set the args passed to BUILD_ARGS

Signed-off-by: cmoulliard <[email protected]>

Remove set +x to understand why the bash script fails

Signed-off-by: cmoulliard <[email protected]>

Remove double quotes around argument: SSH_ARGS and echo BUILD_ARGS

Signed-off-by: cmoulliard <[email protected]>

Remove set +x to understand why the bash script fails

Signed-off-by: cmoulliard <[email protected]>

Declare the SSH_ARGS using an array

Signed-off-by: cmoulliard <[email protected]>

Remove set -x as non needed and fix wrong path to get image_digest

Signed-off-by: cmoulliard <[email protected]>

Remove trailling spaces

Signed-off-by: cmoulliard <[email protected]>

Removing ##### from echo commands

Signed-off-by: cmoulliard <[email protected]>

Generate the SBOM of the base image

Signed-off-by: cmoulliard <[email protected]>

Remove trailing space reported by yamllint

Signed-off-by: cmoulliard <[email protected]>

Remove trailing ##

Signed-off-by: cmoulliard <[email protected]>

Removing the BUILDER_IMAGE parameter

Signed-off-by: cmoulliard <[email protected]>
@cmoulliard
Copy link
Author

Please squash commits into one with nice description

Done @MartinBasti

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants