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

feat: validate kustomize build of tasks and pipelines #1792

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/check-kustomize-build.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: Validate PR - kustomize manifests
'on':
pull_request:
branches: [main]
jobs:
kustomize-build:
name: Check Kustomize Build of Task and Pipelines
runs-on: ubuntu-latest
steps:
- name: Check out code
uses: actions/checkout@v4
- name: Install oc
run: |
set -euo pipefail
url=https://mirror.openshift.com/pub/openshift-v4/x86_64/clients/ocp/latest-4.17/openshift-client-linux.tar.gz
if ! which oc; then
curl --fail --no-progress-meter -L "$url" | gzip -cd | sudo -- tar -x -C /usr/bin oc
fi
- name: Validate Manifests
run: hack/verify-manifests.sh
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ Buildah also has a remote version, which can be generated with:
./hack/generate-buildah-remote.sh
```

## Making changes to tasks and pipelines

If your tasks or pipelines contains `kustomization.yaml`, after making changes to the tasks or pipelines, run `hack/build-manifests.sh` and
commit the generated manifests as well to the same directory (in addition to your changes).
It will help us to make sure the kustomize build of your changes is successful and nothing broken while review the changes.

In CI, `hack/verify-manifests.sh` script will verify that you have submitted the built manifests as well while sending the PR.

## Testing

### Prerequisites
Expand Down
71 changes: 71 additions & 0 deletions hack/build-manifests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#!/bin/bash -e

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

# You can ignore building manifests for some tasks by providing the SKIP_TASKS variable
# with the task name separated by a space, for example:
# SKIP_TASKS="git-clone init"

SKIP_TASKS="generate-odcs-compose provision-env-with-ephemeral-namespace verify-signed-rpms"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why allowing to skip tasks and pipelines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tasks are mirrored from another repo, there is some automation which syncs them. If we change anything manually, that will be reverted. So skipped them.


# You can ignore building manifests for some pipelines by providing the SKIP_PIPELINES variable
# with the task name separated by a space, for example:
# SKIP_PIPELINES="rhtap gitops-pull-request-rhtap"

SKIP_PIPELINES="gitops-pull-request-rhtap"

main() {
local dirs

cd "$SCRIPT_DIR/.."
local ret=0
find task/*/*/*.yaml -maxdepth 0 | awk -F '/' '{ print $0, $2, $3, $4 }' | \
while read -r task_path task_name task_version file_name
do
if [[ "$file_name" == "kustomization.yaml" ]]; then
echo "Building task manifest for: $task_name"
else
continue
fi
# Skip the tasks mentioned in SKIP_TASKS
skipit=
for tname in ${SKIP_TASKS};do
[[ ${tname} == "${task_name}" ]] && skipit=True
done
[[ -n ${skipit} ]] && continue
if ! oc kustomize -o "task/$task_name/$task_version/$task_name.yaml" "task/$task_name/$task_version/"; then
echo "failed to build task: $task_name" >&2
ret=1
fi
done

find pipelines/*/*.yaml -maxdepth 0 | awk -F '/' '{ print $0, $2, $3 }' | \
while read -r pipeline_path pipeline_name file_name
do
if [[ "$file_name" == "kustomization.yaml" ]]; then
echo "Building pipeline manifest for: $pipeline_name"
else
continue
fi
# Skip the pipelines mentioned in SKIP_PIPELINES
skipit=
for pname in ${SKIP_PIPELINES};do
[[ ${pname} == "${pipeline_name}" ]] && skipit=True
done
[[ -n ${skipit} ]] && continue
if ! oc kustomize -o "pipelines/$pipeline_name/$pipeline_name.yaml" "pipelines/$pipeline_name"; then
echo "failed to build pipeline: $pipeline_name" >&2
ret=1
fi
done

exit "$ret"

}

if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
main "$@"
fi



9 changes: 9 additions & 0 deletions hack/missing-ta-tasks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,31 @@ trap 'rm "${tmp_files[@]}" > /dev/null 2>&1' EXIT
# Tasks that are currently missing Trusted Artifact variant
todo=(
task/buildah-10gb/0.2/kustomization.yaml
task/buildah-10gb/0.2/buildah-10gb.yaml
task/buildah-20gb/0.2/kustomization.yaml
task/buildah-20gb/0.2/buildah-20gb.yaml
task/buildah-24gb/0.2/kustomization.yaml
task/buildah-24gb/0.2/buildah-24gb.yaml
task/buildah-6gb/0.2/kustomization.yaml
task/buildah-6gb/0.2/buildah-6gb.yaml
task/buildah-8gb/0.2/kustomization.yaml
task/buildah-8gb/0.2/buildah-8gb.yaml
task/buildah-min/0.2/kustomization.yaml
task/buildah-min/0.2/buildah-min.yaml
task/buildah-rhtap/0.1/buildah-rhtap.yaml
task/download-sbom-from-url-in-attestation/0.1/download-sbom-from-url-in-attestation.yaml
task/fbc-related-image-check/0.1/fbc-related-image-check.yaml
task/fbc-related-image-check/0.2/kustomization.yaml
task/fbc-related-image-check/0.2/fbc-related-image-check.yaml
task/fbc-validation/0.1/fbc-validation.yaml
task/fbc-validation/0.2/kustomization.yaml
task/fbc-validation/0.2/fbc-validation.yaml
task/gather-deploy-images/0.1/gather-deploy-images.yaml
task/generate-odcs-compose/0.2/generate-odcs-compose.yaml
task/generate-odcs-compose/0.2/kustomization.yaml
task/inspect-image/0.1/inspect-image.yaml
task/inspect-image/0.2/kustomization.yaml
task/inspect-image/0.2/inspect-image.yaml
task/operator-sdk-generate-bundle/0.1/operator-sdk-generate-bundle.yaml
task/opm-get-bundle-version/0.1/opm-get-bundle-version.yaml
task/opm-render-bundles/0.1/opm-render-bundles.yaml
Expand Down
20 changes: 20 additions & 0 deletions hack/verify-manifests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash -e

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

main() {

"${SCRIPT_DIR}"/build-manifests.sh
if [[ $(git status --porcelain) ]]; then
git diff --exit-code >&2 || {
echo "Did you forget to build the manifests locally?" >&2;
echo "Please run ./hack/build-manifests.sh and update your PR" >&2;
exit 1;
}
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like ./hack/build-manifests.sh becomes a mandatory part of the development workflow when working with tasks and pipelines. Can you describe this? Probably inside the build-manifests.sh script itself at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Described it in the README change here. Do you think it is sufficient or need to be added to the build-manifests.sh script as well?

Copy link
Contributor

@tkdchen tkdchen Jan 21, 2025

Choose a reason for hiding this comment

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

The content linked tells what need to do for task and pipeline authors, it is not a description to the whole solution. In my mind, the description could tell, for example, what the task and pipeline authors should do, what the maintainers should do, how the kustomized manifests are validated, whether this validation changes the build-definitions task and pipelines contribution workflow, even the build-and-push, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the README now. We don't need to change anything on the build-and-push script.

echo "changes are up to date" >&2
}

if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
main "$@"
fi
Loading
Loading