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

ICU-23056 Add workflow to generate commit checker report #3413

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

echeran
Copy link
Contributor

@echeran echeran commented Feb 22, 2025

Adds a CI workflow that is manually invoked that automates the instructions for the BRS task to generate the commit checker report.

Details:

  • Output: After the workflow completes, the summary page for that workflow instance will render the markdown of the report at the bottom, like so:
    https://github.com/echeran/icu/actions/runs/13466516414
  • Authentication: When the commit range includes a sensitive ticket, the tool will fail unless the user provides the optional needed for this purpose. Otherwise, the credentials are optional.
  • Privacy: The workflow avoids needing to print the user email & API token. In addition, the workflow will mask the user email & API token in the future hypothetical scenario that they are needed in some way that gets captured by the log. (This is a feature after invoking the add-mask directive in Github Actions).

Note: The instructions for running the commit checker tool locally are in the BRS tasks section of the User Guide here: https://unicode-org.github.io/icu/processes/release/tasks/miscellaneous.html#check-integrity-of-jira-issues-in-commit-messages

Checklist

  • Required: Issue filed: ICU-23056
  • Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

# We protect the API token as a secret by preventing its value from pasted at the command line
# by using add-mask to obfuscate secret values in the jobs
# https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions
jira_user_api_token:
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you consider making this a GitHub secret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't suggest making it an input.
Make it a secret, always.
The same way we have the git token to manipulate the release.
Or the gpg password and gpg keys used to sign the release files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good question. The docs from Jira said that an API token cannot last more than 1 year, which seemed short to me. At most, 2 cycles. Creating a user token is fairly easy to do. Also, the Jira API token is tied to a specific user. And a quick search indicates that there's no possibility of a user-neutral service account.

So for all those reasons, I felt like it's not much extra work for people to make their own token when they need it to the run the report.

But do you think it's still a net benefit to make the token into a GH secret, and just have a BRS task only to change out the API token every 6 months? Even though it's tied to a specific person? I could buy that. Just wondering what you think.

type: string
required: true
description: The ICU Jira "Fix Version" semver
from_git_ref:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this to be so flexible, from git-ref to git-ref?
Working with tags would be a lot easier and more readable
(think from release-76-1 to release-77-rc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This derives from how git works. When you issue a command (ex: checkout, reset, etc.), what you provide as an argument is called a git ref. If "ref" is an interface/supertype, then concrete types of a ref include tags, commit hashes, and branch names.

As you know, with git, the implementation is the specification. So I presume the flexibility comes from git.

I also personally don't think the the flexibility is a problem in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that I can use release-76-rc?
And that's a git-ref in GitHub lingo?

In fact in git a tag is basically an alias to a ref, or a pointer to a ref (however you want to look at it)

So maybe it's not really a GitHub lingo, but a git one :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, release-76-rc is a git tag, which is concrete type of a git ref, so it will work. And yes, that's an underlying git thing, not a Github-specific thing.

echo "::add-mask::${JIRA_PASSWORD}"
if [ ! -z "${JIRA_USERNAME}" ] && [ ! -z "${JIRA_PASSWORD}" ]
then
echo "::add-mask::${JIRA_USERNAME}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, not an expert, but this has a bit of "code smell"
When working with GitHub secrets there is a lot of masking done automatically.

I would have to dig a bit for best practices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the links that I pasted in the comments above this step. Within the first link, see this section about add-mask

Copy link
Member

Choose a reason for hiding this comment

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

I think I wrote the script such that environment variables for the secrets would work. I use a dotenv file locally but you don't need to in CI if there is a better option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see instructions about environment variables, But if that's possible, that would make using the Jira API tokens in the GHA workflows even safer via GH tokens.

tools/commit-checker/Pipfile.lock
- name: Install pipenv
run: |
sudo pip3 install pipenv
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't venv do the same kind of things (out of the box, no need to install anything)

I did a very quick scan of several articles and there seems to be agreement that pipenv is more powerful.

But I've also seen several recommendations to start with venv, get familiar with virtual environments, and when/if you need more power switch to pipenv.

More power often means added complexity :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered that out of scope. This is just automating the existing instructions.

I also don't know enough about either to comment, but I would be interested to learn more, assuming that this topic is orthogonal to the objective of the PR.

Copy link
Member

@sffc sffc Feb 22, 2025

Choose a reason for hiding this comment

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

I wrote this script and chose pipenv due to its simplicity and reproducibility. It behaves more like a modern package manager. I've tried many ways to install Python dependencies and this is by far my favorite.

I don't like venv because you make a profile global to the system user. Pipenv makes a profile local to the project directory, as it should be. (I think it uses venv under the hood but what's surfaced to the user is a project-local environment.)

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