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.

Sign up for GitHub

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

Jump to bottom

feat: add image user option #1266

Merged
merged 1 commit into from
Oct 24, 2024
Merged

feat: add image user option #1266

merged 1 commit into from
Oct 24, 2024

Conversation

maxbrunet
Copy link
Contributor

Hello! 👋

When using a base image that does not set a user (e.g. busybox), it is desirable to set a non-root user for the built image. At the moment, this requires building a custom image built with another tool to set this value. Since it is a simple config field, it seems reasonable to have an option for it

@rsrchboy
Copy link
Contributor

This would be incredibly useful for a project I'm working on.

@github-actions GitHub Actions
Copy link

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@maxbrunet
Copy link
Contributor Author

Any chance to get some feedback on this PR? 🙏

@rsrchboy
Copy link
Contributor

rsrchboy commented Oct 2, 2024

I can't do an official review, but going off of CONTRIBUTING (and common sense) things LGTM. Tests are included and pass; --image-user behaves as expected.

CI appears to be complaining about license issues, but apparently the "details" link isn't public, so I don't know what's going on there. Given that this PR does not introduce any library changes, I'm inclined to think that these are preexisting issues.

@imjasonh
Copy link
Member

This change looks good to me! Can you rebase so that CI runs and I can approve and merge? 👍

@imjasonh imjasonh enabled auto-merge (rebase) October 16, 2024 17:14
@maxbrunet
feat: add image user option
f52f41f 
Signed-off-by: Maxime Brunet <[email protected]>
auto-merge was automatically disabled October 18, 2024 00:56

Head branch was pushed to by a user without write access

@maxbrunet
Copy link
Contributor Author

This change looks good to me! Can you rebase so that CI runs and I can approve and merge? 👍

Done! ❤️

@HeavyWombat
Copy link
Contributor

Looking forward to testing this out.

@imjasonh imjasonh merged commit ac22328 into ko-build:main Oct 24, 2024
21 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Oct 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpanato cpanato cpanato approved these changes

@imjasonh imjasonh imjasonh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maxbrunet @rsrchboy @imjasonh @HeavyWombat @cpanato