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: add token input and pin github-server-url #12

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Oct 18, 2024

Description

GHES overwrites github.token. Also default github server for GHES is not http://github.com.

This PR:

  • pin http://github.com as github-server-url for actions/checkout. Required so that actions/checkout correctly finds Trivy repository.
  • add token input. Default value is github.token (as in actions/checkout). token is required so that users can overwrite invalid (for http://github.com server) GHES token. Despite the fact that Trivy is public repository, checkout gives an error when the token is invalid.

Test runs:

Related issues:

@DmitriyLewen
Copy link
Contributor Author

@BAiler-ai confirmed that these changes work for GHES - aquasecurity/trivy-action#418 (comment)

@DmitriyLewen DmitriyLewen marked this pull request as ready for review October 23, 2024 04:51
action.yaml Outdated
Comment on lines 37 to 41
if [ -z "${{ inputs.token }}" ]; then
echo "token=${{ github.token }}" >> $GITHUB_OUTPUT
else
echo "token=${{ inputs.token }}" >> $GITHUB_OUTPUT
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this step if we set the default value of token like actions/checkout? inputs.token points to the default GitHub token if token is not specified.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 23, 2024

Choose a reason for hiding this comment

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

initially, I used ${{ github.token }} as default value for inputs.token.

But actions/checkout doesn't work if token is empty (but works if token is specified) - https://github.com/DmitriyLewen/test-trivy-action/actions/runs/11476162864/job/31935565983

So, there are 2 ways to avoid errors in trivy-action:

  • use ${{ github.token }} as default value for token-setup-trivy in trivy-action.
  • add step in setup-trivy (my way).

I thought it's better to add step in setup-trivy to avoid questions in trivy-action about using ${{ github.token }}.
But if you want, I can use the 1st method.

Example:
setup-trivy (${{ github.token }} is default value for inputs.token) - https://github.com/DmitriyLewen/setup-trivy/blob/0faeae687c92b01123d6eaafbe78091bc8b610eb/action.yaml
trivy-action (`` is default value for inputs.token-setup-trivy)- https://github.com/DmitriyLewen/trivy-action/blob/e78daca2a1cb92de6fd1ba645bd146b851efc10c/action.yaml
test run - https://github.com/DmitriyLewen/test-trivy-action/actions/runs/11476162864/job/31935565983

Copy link
Collaborator

Choose a reason for hiding this comment

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

initially, I used ${{ github.token }} as default value for inputs.token.

What changed your mind to go for the second approach (additional step)? Is there any benefit?

But actions/checkout doesn't work if token is empty (but works if token is specified) - https://github.com/DmitriyLewen/test-trivy-action/actions/runs/11476162864/job/31935565983

I'm probably missing something, but my suggestion will not fall into this case, right?

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 23, 2024

Choose a reason for hiding this comment

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

What changed your mind to go for the second approach (additional step)? Is there any benefit?

I didn't think actions/checkout uses token for public repos.
But IIUC actions/checkout uses same logic as ghcr (you need to use a valid token even if you pull a public image).

That's why I thought trivy-action users would have questions about github.token, how we use it, etc.

So the only benefit would be fewer questions (if any)

I'm probably missing something, but my suggestion will not fall into this case, right?

Since we are using a chain of actions (trivy-action -> setup-trivy -> actions/checkout), we need to set a default value (github.token) for trivy-action and setup-trivy.
If we use an empty default value for trivy-action - setup-trivy will get `` instead of nil and we will get an error.

PS I tried removing default line (I was hoping that would give me nil), but I got the same behavior.

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 now realized that another plus is that we don't have to set the default value twice.
If there is a need to change it, we will do it in one place

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are using a chain of actions (trivy-action -> setup-trivy -> actions/checkout), we need to set a default value (github.token) for trivy-action and setup-trivy.

Yes, I suggested adding default: ${{ github.token }}. It wouldn't be empty, right? What's wrong?

Copy link
Member

Choose a reason for hiding this comment

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

Since we are using a chain of actions (trivy-action -> setup-trivy -> actions/checkout), we need to set a default value (github.token) for trivy-action and setup-trivy.

Yes, I suggested adding default: ${{ github.token }}. It wouldn't be empty, right? What's wrong?

Is it because the default: ${{ github.token }} doesn't propagate down the chain of actions? If it does, I don't see why we shouldn't use it as the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. It seems I have confused myself.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 24, 2024

Choose a reason for hiding this comment

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

README.md Outdated
- name: Install Trivy
uses: aquasecurity/setup-trivy@v0.2.0
with:
version: v0.56.1
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
version: v0.56.1
version: v0.56.2

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 in 34d9f0e

@DmitriyLewen DmitriyLewen merged commit e71d901 into aquasecurity:main Oct 24, 2024
1 check passed
@DmitriyLewen DmitriyLewen deleted the fix-ghes branch October 24, 2024 05:30
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.

Setup action v0.2.1 failing on GitHub Enterprise Server
3 participants