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

fix(KFLUXBUGS-1215): prefetch: add git auth support and logging #944

Closed
wants to merge 3 commits into from

Conversation

scoheb
Copy link
Contributor

@scoheb scoheb commented Apr 17, 2024

  • needed for private repos
  • add ability to turn on cachi2 debug logging

fixes KFLUXBUGS-1215

@scoheb scoheb force-pushed the prefetch-gitauth branch from ebdc1a2 to 1685757 Compare April 17, 2024 20:07
@scoheb scoheb changed the title prefetch: add git auth support and logging fix(KFLUXBUGS-1215): prefetch: add git auth support and logging Apr 18, 2024
|---|-----------------------------------------------------------------------------------------------------------------------------------------------------|---|---|
|input| Configures project packages that will have their dependencies prefetched. ||true|
|dev-package-managers| Enable in-development package managers. WARNING: the behavior may change at any time without notice. Use at your own risk. |false|false|
|enable-debug-logging| Enable debug logging with cachi2 |false|false|
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this option. Could we make it more generic?

Suggested change
|enable-debug-logging| Enable debug logging with cachi2 |false|false|
|log-level| Set cachi2 log level |info|false|

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

I'm concerned with the amount of logic this copy-pastes from the clone task. Do we really need # Compatibility with kubernetes.io/basic-auth secrets? If not, this would just be a cp and chmod of two files.

If we do, then oh well. Let's call this a limitation of Tekton 🥲

Comment on lines 96 to 97
# needed or else you'll see "could not read Username for 'https://gitlab.com':"
cd $(workspaces.source.path)/source && git config remote.origin.url $(cat "${PARAM_USER_HOME}/.git-credentials")
Copy link
Contributor

@chmeliik chmeliik Apr 18, 2024

Choose a reason for hiding this comment

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

This doesn't seem right, the clone task doesn't need it.

I think this approach may also print credentials in the task logs in some cases (IIUC that's why #861 was needed, unfortunately can't tell anymore from the description)

Copy link
Contributor

Choose a reason for hiding this comment

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

And cachi2 might include the credentials in the SBOM, it uses the origin url to construct the purl of packages coming from the source repo

Copy link
Contributor

Choose a reason for hiding this comment

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

And cachi2 might include the credentials in the SBOM, it uses the origin url to construct the purl of packages coming from the source repo

Filed containerbuildsystem/cachi2#521

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 have altered my approach. please review

scoheb added 3 commits April 18, 2024 17:00
- needed for private repos
- add ability to turn on cachi2 debug logging

fixes KFLUXBUGS-1215

Signed-off-by: Scott Hebert <[email protected]>
Signed-off-by: Scott Hebert <[email protected]>
@scoheb scoheb force-pushed the prefetch-gitauth branch from 844c796 to 04a33a5 Compare April 18, 2024 21:00
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@chmeliik
Copy link
Contributor

There's no need to do this differently than the clone task. I've tested it and opened #953

@chmeliik
Copy link
Contributor

There's no need to do this differently than the clone task. I've tested it and opened #953

If that looks reasonable, let's keep only the logging option here

@chmeliik chmeliik self-requested a review April 19, 2024 12:35
@scoheb
Copy link
Contributor Author

scoheb commented Apr 19, 2024

Closed in favour of #953

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.

3 participants