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

Create Tekton task and Pipeline for Tekton bundles #284

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

zregvart
Copy link
Member

This adds tkn-bundle Tekton Task that simply finds all *.yaml or *.yml files in the current params.CONTEXT directory and creates a Tekton Bundle by pushing it to params.IMAGE. The result of the task are IMAGE_URL image reference with digest (no tag) and IMAGE_DIGEST containing the image digest.

Also adds tekton-bundle-builder builder pipeline that uses the tkn-bundle task for the build step.

Tests are provided.

Ref. https://issues.redhat.com/browse/HACBS-1675

Copy link
Contributor

@robnester-rh robnester-rh left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@simonbaird simonbaird left a comment

Choose a reason for hiding this comment

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

Approve but consider my suggestion about the docs and extra comments.

@simonbaird
Copy link
Contributor

I guess the tests might go stale unless there's some automation to run them. Would it be crazy to add a GitHub workflow in this repo with the kind cluster setup to exercise them?

@zregvart
Copy link
Member Author

I guess the tests might go stale unless there's some automation to run them. Would it be crazy to add a GitHub workflow in this repo with the kind cluster setup to exercise them?

Entirely possible, let's do this as a followup: HACBS-1698

@zregvart zregvart force-pushed the issue/HACBS-1675 branch 4 times, most recently from dd49758 to 94b327b Compare January 27, 2023 12:46
@zregvart
Copy link
Member Author

Something is not setup correctly on redhat-appstudio-tekton-catalog/pull-request-builds, perhaps it is not public?

$ curl https://quay.io/api/v1/repository/redhat-appstudio-tekton-catalog/pull-request-builds:nodejs-builder-94b327bfea218b7ff71ef0a0cc73cbc278c8e888 
{"detail": "Requires authentication", "error_message": "Requires authentication", "error_type": "invalid_token", "title": "invalid_token", "type": "https://quay.io/api/v1/error/invalid_token", "status": 401}

vs.

$ curl https://quay.io/api/v1/repository/redhat-appstudio-tekton-catalog/pipeline-nodejs-builder                                                     
{"namespace": "redhat-appstudio-tekton-catalog", "name": "pipeline-nodejs-builder", "kind": "image", "description": "", "is_public": true,...

Also not sure why the tkn-bundle task is pushed with tkn-bundle-0.1-237b8766e22ddb3376f391d825adf511cb4a8bb6 and tkn-bundle-0.1 tags.

@zregvart
Copy link
Member Author

Oh, I think I get it

@zregvart zregvart force-pushed the issue/HACBS-1675 branch 4 times, most recently from a3b8b66 to e17bb0a Compare January 30, 2023 13:54
@zregvart
Copy link
Member Author

The check is now showing green. @Michkov and @tkdchen I would appreciate your feedback.

pipeline-bundle-list Outdated Show resolved Hide resolved
.tekton/pull-request.yaml Outdated Show resolved Hide resolved
patches:
# Use the template-build as a template replacing the Pipeline name and the
# `build-container` step's task reference
- patch: |-
Copy link
Contributor

@lcarva lcarva Jan 30, 2023

Choose a reason for hiding this comment

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

NIT: Maybe worth setting these labels? pipelines.openshift.io/runtime, pipelines.openshift.io/strategy?

(although... I'm not quite sure how those are used)

Copy link
Member Author

Choose a reason for hiding this comment

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

Found no documentation on this, no way of telling what those need to be set to so I left them unset

# Use the template-build as a template replacing the Pipeline name and the
# `build-container` step's task reference
- patch: |-
- op: replace
Copy link
Contributor

@lcarva lcarva Jan 30, 2023

Choose a reason for hiding this comment

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

NIT: It might be worth loading a patch from its own yaml (like the other pipelines do). It's a bit easier to edit than an embedding yaml in yaml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, this documents the patch much better

task/tkn-bundle/0.1/tkn-bundle.yaml Outdated Show resolved Hide resolved
task/tkn-bundle/0.1/tkn-bundle.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@Michkov Michkov left a comment

Choose a reason for hiding this comment

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

I am slightly confused about purpose of this pipeline, who is going to use it? Will it be provided to customers?

&& exit 1
exec 3>&1;
OUT="$(tkn bundle push "$(params.IMAGE)" \
$(printf ' -f %s' "${FILES[@]}") \
Copy link
Contributor

Choose a reason for hiding this comment

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

limitation of 10?

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO, this is something that needs to be dealt with in Tekton. The task will fail with more than 20 (see this PR). I don't think it is worth taking additional complexity.

task/tkn-bundle/OWNERS Outdated Show resolved Hide resolved
task/tkn-bundle/0.1/tkn-bundle.yaml Outdated Show resolved Hide resolved
@zregvart
Copy link
Member Author

I am slightly confused about purpose of this pipeline, who is going to use it? Will it be provided to customers?

This is a requirement for implementing SLSA build as code requirement (see HACBS-1282). We need to start somewhere and with time migrate or adapt our own pipelines in this repository to generate provenance information.

So, this is for customers who want to be SLSA level 3, and for the pipelines/tasks from this repository if we chose to do so as well.

.tekton/pull-request.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@tkdchen tkdchen left a comment

Choose a reason for hiding this comment

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

The CI failed as commented above the oc command does not exist.

@zregvart zregvart force-pushed the issue/HACBS-1675 branch 2 times, most recently from 4353ed5 to ff6b6ef Compare February 1, 2023 10:15
This adds `tkn-bundle` Tekton Task that simply finds all *.yaml or *.yml
files in the current `params.CONTEXT` directory and creates a Tekton
Bundle by pushing it to `params.IMAGE`. The result of the task are
`IMAGE_URL` image reference with digest (no tag) and `IMAGE_DIGEST`
containing the image digest.

Also adds `tekton-bundle-builder` builder pipeline that uses the
`tkn-bundle` task for the `build` step.

Tests are provided.

Ref. https://issues.redhat.com/browse/HACBS-1675
Passes the pull request commit id to the
`check-task-pipeline-repo-existence` step to make the validation
consider pull request image references; otherwise the check checks for
images that are have resulted from changes pushed to the default branch.

The URL being checked for existence now checks against the Docker
registry API, the quay.io REST API has no endpoint to check for
existence of tags. Also the non-pull request artifacts are checked for
their correct tag/version now.
@zregvart
Copy link
Member Author

zregvart commented Feb 1, 2023

Last pipeline run was successful. I want to do one more quality of life change: for faster feedback it would be better if check-task-pipeline-repo-existence ran after build-container and not in finally. Let me try to do that and then I think this should be okay to merge. Though do check for any bash issues, those are always lurking.

@zregvart
Copy link
Member Author

zregvart commented Feb 1, 2023

@tkdchen @Michkov @lcarva can you have another look at this?

FYI @Michkov looks like registry.redhat.io/openshift4/ose-tools-rhel8 contains good amount of tools we usually need (oc, git, jq, bash...)

@zregvart
Copy link
Member Author

zregvart commented Feb 1, 2023

Thanks for the reviews, merging now...

@zregvart zregvart merged commit d44f241 into konflux-ci:main Feb 1, 2023
@zregvart zregvart deleted the issue/HACBS-1675 branch February 1, 2023 15:52
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