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

Add create release branch action #59

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

Conversation

blink1073
Copy link
Member

@blink1073 blink1073 requested a review from alcaeus October 9, 2024 14:22
> You will need to wait overnight before making a release on
> the new branch to allow Silk to be populated, so it is recommended to
> make a minor/major release prior to creating a release branch, or create the
> release branch at least one day before a planned release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to confirm, this action is intended to be run in preparation for a release, not as part of the release process itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct

Comment on lines +29 to +30
silk_group_prefix:
description: The prefix to use for the silk asset group, defaults to the repo name
Copy link
Collaborator

Choose a reason for hiding this comment

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

It makes sense that we use a prefix here since we'll now have to include the branch name into the silk group. However, the sbom group takes a silk_asset_group, so if nothing else we should document how these two actions will play nice together in future.


runs:
using: composite
steps:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I like the flow of creating a branch and updating version info. For example, on PHP we have v1.x as a development branch for the next minor version, and v1.19.x as a maintenance branch. However, v1.x already has version info for the next minor version (so 1.20.0-dev at the moment), and when we branch off for 1.20 we would update v1.x to 1.21.0-dev. Could you explain the branch management scheme that you envision for this script?

Copy link
Member Author

Choose a reason for hiding this comment

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

For release branches in Go and Python, we would release from the main branch which is targeting the next minor release, then create a release branch for that minor version. The release branch's version would target the next patch version, so 1.20.1-dev in your example.

Comment on lines +30 to +43
json_payload=$(cat <<EOF
{
"active": true,
"name": "${SILK_GROUP}",
"code_repo_url": "https://github.com/${OWNER_REPO}",
"branch": "${BRANCH}",
"metadata": {
"sbom_lite_path": "${SBOM_FILE_PATH}"
},
"file_paths": [],
"asset_id": "$SILK_GROUP"
}
EOF
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume that this config works for all of our repos; if not we can always make this customisable or use a template asset group that we fetch and modify.

Copy link
Member Author

Choose a reason for hiding this comment

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

This config works for all repos that I maintain, including libmonocrypt/bindings/python, but yeah, we could make it templatable if needed.

Comment on lines +52 to +68
echo "Create a temp sbom."
TMP_SBOM=sbom-for-${BRANCH}.json
podman run --platform="linux/amd64" --rm -v "$(pwd)":/pwd \
${ARTIFACTORY_IMAGE}/silkbomb:1.0 \
update --sbom-out /pwd/${TMP_SBOM}

echo "Get the new timestamp and serial number."
set -x
SERIAL=$(jq -r '.serialNumber' ${TMP_SBOM})
TIMESTAMP=$(jq -r '.metadata.timestamp' ${TMP_SBOM})
rm ${TMP_SBOM}

cat ${SBOM_FILE_PATH}
echo "Replace the values in the existing sbom."
cat <<< "$(jq --indent 4 '.serialNumber = "'${SERIAL}'"' ${SBOM_FILE_PATH})" > ${SBOM_FILE_PATH}
cat <<< "$(jq --indent 4 '.metadata.timestamp = "'${TIMESTAMP}'"' ${SBOM_FILE_PATH})" > ${SBOM_FILE_PATH}
cat ${SBOM_FILE_PATH}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure I follow exactly what's happening here. We fetch the SBOM for the new branch, then bump the serial number in the SBOM file path - why is that?

Copy link
Member Author

Choose a reason for hiding this comment

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

The SBOM needs to be unique per branch if I understand correctly.

Comment on lines +70 to +72
echo "Update the workflow with the silk asset group and evergreen project."
sed -i 's/SILK_ASSET_GROUP:.*/SILK_ASSET_GROUP: '${SILK_GROUP}'/' ${RELEASE_WORKFLOW_PATH}
sed -i 's/EVERGREEN_PROJECT:.*/EVERGREEN_PROJECT: '${EVERGREEN_PROJECT}'/' ${RELEASE_WORKFLOW_PATH}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this relates to my earlier comment about the asset group prefix vs. hardcoded asset name. I'd personally change the release tooling to use the same prefix logic for the silk asset group.
As for evergreen project name, we don't use that in our PHP release workflow, but I can see how other drivers may need it. Wouldn't it make more sense to use the same <prefix>-<branch-name> for those names so we don't need to update this information for each release branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use the evergreen project name to tie the release to an Evergreen run url, which is included in the compliance report. Doesn't each branch need its own Evergreen project? Either way, we could make this part optional if no evergreen project is given.

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.

Add New Branch Action
2 participants