feat(transparent-proxy): allow configure transparent proxy from config file #11089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bartsmykla
added
the
ci/run-full-matrix
bartsmykla
force-pushed
the
feat/allow-configure-tproxy-from-file
branch
3 times, most recently
from
8b6daf7
to
d2f2743
Compare
The name `Comments` is more appropriate than `Comment`. Signed-off-by: Bart Smykla <[email protected]>
This feature prepares for upcoming changes that use this function to load transparent proxy config files. Signed-off-by: Bart Smykla <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
This change ensures consistency with the rest of the transparent proxy config structures. Signed-off-by: Bart Smykla <[email protected]>
This change removes unnecessary parsing and assigns the value directly during flag parsing by the Cobra library. Signed-off-by: Bart Smykla <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
This will be used for parsing config files Signed-off-by: Bart Smykla <[email protected]>
This will be used for parsing config files Signed-off-by: Bart Smykla <[email protected]>
These abstractions allow setting values during Cobra flag parsing and will be used for config file parsing in upcoming commits. Signed-off-by: Bart Smykla <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
Introduced a `Loader` struct with additional methods to support loading configurations from various sources (file, raw content, or `io.Reader`) Signed-off-by: Bart Smykla <[email protected]>
The `*Ports.Set` method is also invoked in `*Ports.UnmarshalJSON`, which can lead to situations where config is parsed from JSON/YAML and then modified by a CLI flag, causing values to merge instead of overwrite. To ensure consistency, the slice should be cleared before setting new values. Signed-off-by: Bart Smykla <[email protected]>
The expected order of precedence was not being preserved, with configs provided via `--transparent-proxy-config-file` or `--transparent-proxy-config` flags incorrectly taking the highest priority. The correct order of precedence, from lowest to highest, is: 1. Config File (via `--transparent-proxy-config` or `--transparent-proxy-config-file` flags) 2. Environment Variables 3. CLI Flags Signed-off-by: Bart Smykla <[email protected]>
Manually handle the `--help` flag since `DisableFlagParsing` is enabled, requiring custom flag parsing to maintain the correct order of precedence. To avoid unnecessary logic execution when `--help` is invoked, moved the parsing of config-related flags into the `RunE` method and removed the `PreRunE` method. Signed-off-by: Bart Smykla <[email protected]>
bartsmykla
force-pushed
the
feat/allow-configure-tproxy-from-file
branch
from
d2f2743
to
fbfbeec
Compare
…ure-tproxy-from-file
The transparent proxy does not rely on any of the features which are being initialized in `PersistentPreRunE` method in the `kumactl` root command. Since it handles its own flag parsing, processing the parent flags like `--config-file` is unnecessary and would add unnecessary complexity Signed-off-by: Bart Smykla <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
Since we’ve confirmed that the `--config-file` flag from the root `kumactl` command isn’t parsed by `kumactl install transparent-proxy`, we can safely use the same name for our flags Signed-off-by: Bart Smykla <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
…ure-tproxy-from-file
Signed-off-by: Bart Smykla <[email protected]>
…ure-tproxy-from-file
…ure-tproxy-from-file
To ensure durations are marshaled as readable strings (e.g., "2s")
instead of nanoseconds ("2000000000") in the kuma-init container, we've
updated sleepBetweenTries to use a custom duration type
Signed-off-by: Bart Smykla <[email protected]>
Added `omitempty` where necessary and the property names starting with lower case where necessary Signed-off-by: Bart Smykla <[email protected]>
Renamed `Owner` to `KumaDPUser` for consistency with the CLI flag (`--kuma-dp-user`) and to improve clarity. Signed-off-by: Bart Smykla <[email protected]>
…ure-tproxy-from-file
Icarus9913
reviewed
Aug 21, 2024
jijiechen
reviewed
Aug 21, 2024
jijiechen
reviewed
Aug 21, 2024
jijiechen
approved these changes
Aug 21, 2024
|
Could you please add a precedence explanation for the flags, ENV, config-files in the |
Icarus9913
approved these changes
Aug 21, 2024
…ure-tproxy-from-file
Signed-off-by: Bart Smykla <[email protected]>
Unfortunately it's currently not trivial to do because of the library we are using to parse this stuff. We'll have to update our docs at the beginning. Maybe I'll have some time later to figure this out |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements MADR: Streamlining Transparent Proxy Configuration (Phase 1): Enable Configuration via Config Files and Environment Variables
Note
Since this PR includes several changes that were necessary to introduce the described functionality, I recommend reviewing it commit by commit
Checklist prior to review
syscall.Mkfifohave equivalent implementation on the other OSci/labels to run additional/fewer testsUPGRADE.md?