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(STONEINTG-1020): make snyk scan all files #1296

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

jsztuka
Copy link
Contributor

@jsztuka jsztuka commented Aug 14, 2024

This change main goal is to make Snyk scan dependencies to meet ProdSec scanning requirements.
It is a breaking change, thats the reason for newer version of the check itself.

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

@jsztuka jsztuka force-pushed the STONEING-1020 branch 7 times, most recently from 205b2a3 to 1dbcdee Compare August 14, 2024 13:14
@jsztuka jsztuka force-pushed the STONEING-1020 branch 2 times, most recently from 6293a79 to e60b4be Compare August 20, 2024 08:24
Copy link
Contributor

@chipspeak chipspeak left a comment

Choose a reason for hiding this comment

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

LGTM

@jsztuka jsztuka force-pushed the STONEING-1020 branch 10 times, most recently from 94926e8 to 7395321 Compare August 20, 2024 12:40
@jsztuka
Copy link
Contributor Author

jsztuka commented Aug 20, 2024

/retest

@jsztuka
Copy link
Contributor Author

jsztuka commented Aug 20, 2024

Duno why konflux is unable to pull docker.io/cytopia/yamllint:1.26@sha256:1bf8270a671a2e5f2fea8ac2e80164d627e0c5fa083759862bbde80628f942b2

@chmeliik
Copy link
Contributor

Duno why konflux is unable to pull docker.io/cytopia/yamllint:1.26@sha256:1bf8270a671a2e5f2fea8ac2e80164d627e0c5fa083759862bbde80628f942b2

I've seen this in other PRs as well, we may have hit the docker.io rate limit 😢

@dheerajodha
Copy link
Member

I've seen this in other PRs as well, we may have hit the docker.io rate limit 😢

@chmeliik how do we fix this? 😢 or do we need to wait till the limit is reset?

@lcarva
Copy link
Contributor

lcarva commented Aug 20, 2024

I think this will work:

  1. Create a Secret in the konflux-ci namespace that contains a token with read access to docker.io. This token doesn't necessarily need to be from a paid account. Any token, instead of anonymous pulls, has a significantly higher rate limit.
  2. Link the Secret to the appstudio-pipeline ServiceAccount with --for=mount,pull.

@chmeliik
Copy link
Contributor

I think this will work:

1. Create a Secret in the `konflux-ci` namespace that contains a token with read access to docker.io. This token doesn't necessarily need to be from a paid account. Any token, instead of anonymous pulls, has a significantly higher rate limit.

2. Link the Secret to the `appstudio-pipeline` ServiceAccount with `--for=mount,pull`.

+1, if you're going to do this please do so via ExternalSecrets (like https://github.com/redhat-appstudio/infra-deployments/blob/main/components/konflux-ci/production/redhat-appstudio-tekton-catalog-build-definitions-pull-secret.yaml)

Or wait until tomorrow for the build team to come back online

@lcarva
Copy link
Contributor

lcarva commented Aug 20, 2024

+1, if you're going to do this please do so via ExternalSecrets (like https://github.com/redhat-appstudio/infra-deployments/blob/main/components/konflux-ci/production/redhat-appstudio-tekton-catalog-build-definitions-pull-secret.yaml)

Don't worry. I have no access here 🥲

@jsztuka
Copy link
Contributor Author

jsztuka commented Aug 21, 2024

/retest

@jsztuka
Copy link
Contributor Author

jsztuka commented Aug 21, 2024

Looks like that worked, thanks @chmeliik !

@jsztuka jsztuka requested review from chmeliik and dirgim August 21, 2024 08:35
@chmeliik
Copy link
Contributor

Looks like that worked, thanks @chmeliik !

Just for the record, we didn't do anything 😄

The rate limit probably just reset

@jsztuka jsztuka force-pushed the STONEING-1020 branch 6 times, most recently from beff70c to 717372a Compare August 21, 2024 09:19
Copy link
Contributor

@dirgim dirgim left a comment

Choose a reason for hiding this comment

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

LGTM

@jsztuka jsztuka added this pull request to the merge queue Aug 21, 2024
Merged via the queue into konflux-ci:main with commit 53705a6 Aug 21, 2024
13 checks passed
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.

7 participants