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

Attach helper script #133

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Attach helper script #133

merged 2 commits into from
Dec 4, 2024

Conversation

arewm
Copy link
Member

@arewm arewm commented Dec 3, 2024

In order to make oras attach operations easier, an attach script has
been added with a modified interface. This includes:

  • Auto-selecting the oci auth for repository-specific tokens
  • Use of a common artifactType
  • Storing the digest of the produced manifest

An additional helper script is added, get-reference-base.sh, to provide
a common way to remove tags and digests from OCI object references.

This PR is an alternative implementation for #130. Only one of the two needs to be approved and merged.

@arewm arewm requested a review from ralphbean as a code owner December 3, 2024 17:09
@arewm arewm force-pushed the attach-helper-script branch 6 times, most recently from 4b5271e to 6c4adc3 Compare December 3, 2024 23:43
@arewm arewm requested review from zregvart and lcarva December 3, 2024 23:45
arewm added 2 commits December 3, 2024 18:45
In order to make oras attach operations easier, an attach script has
been added with a modified interface. This includes:
* Auto-selecting the oci auth for repository-specific tokens
* Use of a common artifactType
* Storing the digest of the produced manifest

An additional helper script is added, get-reference-base.sh, to provide
a common way to remove tags and digests from OCI object references.

Signed-off-by: arewm <[email protected]>
@arewm arewm force-pushed the attach-helper-script branch from 6c4adc3 to 902cfe9 Compare December 3, 2024 23:45
@arewm arewm mentioned this pull request Dec 4, 2024
Copy link
Member

@zregvart zregvart left a comment

Choose a reason for hiding this comment

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

The usual nitpickery, LGTM

# contains pairs of artifacts to attach and (optionally) paths to output the blob digest
artifacts=()
artifact_type="application/vnd.konflux-ci.attached-artifact"
# distribution_spec="v1.1-referrers-api"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# distribution_spec="v1.1-referrers-api"

Copy link
Member Author

Choose a reason for hiding this comment

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

I intentionally left this comment in as I might want to re-enable this default in the future.

Comment on lines +117 to +118
oras attach "${oras_opts[@]}" --no-tty --registry-config <(select-oci-auth ${subject}) --artifact-type "${artifact_type}" \
"${use_distribution_spec[@]}" "${subject}" "${file_name}:${media_type}" | tail -n 1 | cut -d: -f3 > "${digest_file}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
oras attach "${oras_opts[@]}" --no-tty --registry-config <(select-oci-auth ${subject}) --artifact-type "${artifact_type}" \
"${use_distribution_spec[@]}" "${subject}" "${file_name}:${media_type}" | tail -n 1 | cut -d: -f3 > "${digest_file}"
oras attach "${oras_opts[@]}" --no-tty --registry-config <(select-oci-auth ${subject}) --artifact-type "${artifact_type}" \
"${use_distribution_spec[@]}" "${subject}" "${file_name}:${media_type}" --format go-template --template '{{.digest}}' > "${digest_file}"

Comment on lines +18 to +38
original_ref="$1"

# Trim off digest
repo="$(echo -n $original_ref | cut -d@ -f1)"
if [[ $(echo -n "$repo" | tr -cd ":" | wc -c | tr -d '[:space:]') == 2 ]]; then
# format is now registry:port/repository:tag
# trim off everything after the last colon
repo=${repo%:*}
elif [[ $(echo -n "$repo" | tr -cd ":" | wc -c | tr -d '[:space:]') == 1 ]]; then
# we have either a port or a tag so inspect the content after
# the colon to determine if it is a valid tag.
# https://github.com/opencontainers/distribution-spec/blob/main/spec.md
# [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127} is the regex for a valid tag
# If not a valid tag, leave the colon alone.
if [[ "$(echo -n "$repo" | cut -d: -f2 | tr -d '[:space:]')" =~ ^([a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})$ ]]; then
# We match a tag so trim it off
repo=$(echo -n "$repo" | cut -d: -f1)
fi
fi

echo -n "$repo"
Copy link
Member

Choose a reason for hiding this comment

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

This is how I would do it to run within bash only

Suggested change
original_ref="$1"
# Trim off digest
repo="$(echo -n $original_ref | cut -d@ -f1)"
if [[ $(echo -n "$repo" | tr -cd ":" | wc -c | tr -d '[:space:]') == 2 ]]; then
# format is now registry:port/repository:tag
# trim off everything after the last colon
repo=${repo%:*}
elif [[ $(echo -n "$repo" | tr -cd ":" | wc -c | tr -d '[:space:]') == 1 ]]; then
# we have either a port or a tag so inspect the content after
# the colon to determine if it is a valid tag.
# https://github.com/opencontainers/distribution-spec/blob/main/spec.md
# [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127} is the regex for a valid tag
# If not a valid tag, leave the colon alone.
if [[ "$(echo -n "$repo" | cut -d: -f2 | tr -d '[:space:]')" =~ ^([a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})$ ]]; then
# We match a tag so trim it off
repo=$(echo -n "$repo" | cut -d: -f1)
fi
fi
echo -n "$repo"
# 1. anything not slash or colon
# 2. colon and numbers (optional)
# slash
# 3. anything but colon and at symbol
# 4. everything else
imageref_rx='^([^/:]+)(:[[:digit:]]+)?/([^:@]+)(.*)$'
if [[ "$1" =~ ${imageref_rx} ]]; then
host="${BASH_REMATCH[1]}"
port="${BASH_REMATCH[2]}"
repository="${BASH_REMATCH[3]}"
echo "${host}${port}/${repository}"
fi

Copy link
Member Author

Choose a reason for hiding this comment

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

([^/:]+)(:[[:digit:]]+)? is what matches the host, but isn't that is split across two of the rematches?

@arewm
Copy link
Member Author

arewm commented Dec 4, 2024

Merging this PR after approvals to unblock work in build-definitions. We can iterate on the refinement/nitpicks in future PRs.

@arewm arewm merged commit 7830ef6 into konflux-ci:main Dec 4, 2024
3 checks passed
mmalina added a commit to mmalina/release-service-utils that referenced this pull request Dec 12, 2024
`select-oci-auth` stopped working when the oras image ref
was updated to the latest one.

The `select-oci-auth` in the oras-container repo was refactored
and part of the logic was moved to a new `get-reference-base`
script, which it uses. So we need to add that script.

Related PR:
konflux-ci/oras-container#133

Signed-off-by: Martin Malina <[email protected]>
mmalina added a commit to konflux-ci/release-service-utils that referenced this pull request Dec 12, 2024
`select-oci-auth` stopped working when the oras image ref
was updated to the latest one.

The `select-oci-auth` in the oras-container repo was refactored
and part of the logic was moved to a new `get-reference-base`
script, which it uses. So we need to add that script.

Related PR:
konflux-ci/oras-container#133

Signed-off-by: Martin Malina <[email protected]>
@mmalina
Copy link

mmalina commented Dec 12, 2024

This broke release-service-utils. Fixed today here: konflux-ci/release-service-utils#341

@arewm arewm deleted the attach-helper-script branch December 12, 2024 14:07
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.

4 participants