Skip to content

[Tigron]: command env WhiteList support and cosmetic changes #4077

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

Merged
merged 2 commits into from
Apr 14, 2025

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Apr 3, 2025

<!> Blocked by <!>:


This is a new series of Tigron PRs.

Now that the big change with the new command implementation is in, upcoming work on Tigron will focus on:

  • P0: internal testing, getting to the point we can absolutely trust Tigron to never faceplant
  • P0: debugging output readability and other developers quality of life improvements
  • P1: code cleanup and simplification

Ideally, none of these PRs will require ANY change in nerdctl, and are either changes to the way we test Tigron itself, or refactoring that does not change signatures and API.

Efforts are being made to break these PRs down into small, digestible changes to ease and fasten review, which explains the chain of PRs.

LMK at any point if a given PR needs to be further broken down.


This second PR is very small:

Commit 1 adds support for environment whitelisting for internal/com.
The point here is to make the output on the CI a lot less crowded (since we do output the command environment, there is a lot of irrelevant stuff in there - ANDROID stuff, etc - and blacklisting one by one is a wild goose chase).

Second commit is cosmetic / refining linting.

@apostasie apostasie marked this pull request as ready for review April 3, 2025 21:19
@apostasie apostasie changed the title [BLOCKED] [Tigron]: command env WhiteList support and cosmetic changes [STACKED] [Tigron]: command env WhiteList support and cosmetic changes Apr 3, 2025
@apostasie apostasie force-pushed the tigron-2-debugging branch from 6a34138 to c6d4dc9 Compare April 8, 2025 15:54
@apostasie
Copy link
Contributor Author

Rebased.

@apostasie apostasie force-pushed the tigron-2-debugging branch from c6d4dc9 to a35b429 Compare April 12, 2025 15:34
@apostasie apostasie changed the title [STACKED] [Tigron]: command env WhiteList support and cosmetic changes [Tigron]: command env WhiteList support and cosmetic changes Apr 12, 2025
@apostasie
Copy link
Contributor Author

Rebased. Ready for review.

@apostasie
Copy link
Contributor Author

@AkihiroSuda AkihiroSuda added this to the v2.0.5 milestone Apr 12, 2025
Prior, only BlackList-ing of environment variables was supported, and solely for exact variable names match, or "*" for all.

This changeset introduces:
- WhiteList-ing
- prefix matching for env var names (eg: "THING_*" can now be used to white/black list any env variable which name starts with THING_)

Note that whitelisting takes precedence over blacklisting if both are used.

Signed-off-by: apostasie <[email protected]>
@apostasie apostasie force-pushed the tigron-2-debugging branch from a35b429 to 1e0b404 Compare April 13, 2025 17:53
@apostasie
Copy link
Contributor Author

Thanks for the review @AkihiroSuda. Appreciated.

@apostasie apostasie requested a review from AkihiroSuda April 13, 2025 18:02
This is mostly a cosmetic changeset:
- refined golangcilint, explicitly calling which linters we really care about, along with settings for revive
- comments on reasons for some of //nolint
- comments line breaks
- additional FIXME information
- assert type check fixes
- const-ification
- overall making linter happy(-ier)

Signed-off-by: apostasie <[email protected]>
@apostasie apostasie force-pushed the tigron-2-debugging branch from 1e0b404 to 734f7a1 Compare April 13, 2025 18:34
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit d3cb96e into containerd:main Apr 14, 2025
35 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.

2 participants