-
Notifications
You must be signed in to change notification settings - Fork 593
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
support for rpk profile #10528
Merged
Merged
support for rpk profile #10528
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
twmb
force-pushed
the
rpk-context
branch
4 times, most recently
from
May 9, 2023 04:15
e73ac86
to
07c27b0
Compare
twmb
force-pushed
the
rpk-context
branch
2 times, most recently
from
May 9, 2023 14:50
b7ec576
to
7658962
Compare
These settings are passed directly to redpanda; they are seastar flags. We will not need them in the new rpk yaml.
…anda init init was deprecated a while ago, it now officially does actually nothing besides writing the default file, since the node uuid no longer exists.
For the incoming RpkFile function
We should only create the config directory if the user explicitly wants us to, which is now done in PreserveUnixOnwership. We should not create it on read.
Format existed to differentiate between json and yaml; since yaml is a superset of json, we can just always use the yaml decoder and document yaml or json are supported formats.
We use a generic to try to enforce that the input value is a pointer-to-a-struct. This will be used in `rpk context set`.
The new name is clearer -- I kept forgetting what ConfigPath is which made me unsure of what it was for
* Searching in a home directory is non-standard behavior for CLI tools, so we drop it. * We now search the current directory _first_ rather than /etc/redpanda/redpanda.yaml. Odds are a person wants to use what is local to them before what's global to the system. * We now stop adding rpk.yaml by default if a person is trying to locate the redpanda config specifically -- i.e., rpk cluster config lint.
For backcompat, we still deserialize truststore_file, but ca_file is more understandable since truststore is a Java concept.
Next commits begin wiring in using this (and probably modify this a little bit). readConfig has been simplified to always fill in fileLocation by default, which makes it easier to wire in reading rpk.yaml as well. LocateRedpandaConfig has been removed; this is only used in cluster config lint, and loading the configuration itself loads the file location -- we can just use cfg.FileLocation.
For use in rpk, we can use this to make it easier to add default ports.
The Config type has been renamed to RedpandaYaml, and a new Config type exists. This makes it easier to reason about how using materialized configuration. The next commit will wire this through the rest of rpk and fix any breakages / introduce APIs as needed to fix things.
No deleted tests, just pure updates.
Breakage to be fixed in follow up commits
* Adds support for -X cloud.client_id and -X cloud.client_secret, both config knobs that will replace --client-id and --client-secret eventually * We do not require both of these together unlike the current cloud auth, which is ultimately fine -- we just are a bit less strict, but so long as the user provides the right combo, they will succeed * Adds some helpers to structs * Improves the ActualOr functions, we now return the same pointer after the first initialization * Adds the concept of "DevOverrides", which are environment driven development overrides that are not meant to be used by end users. The last part that remains is backcompat support for __cloud.yaml, which will come in a later commit.
This is currently hardcoded within oauth/providers/auth0, but it is meant for cloudapi.
* We now use config.Config, differentiating between actual and materialized rpk as appropriate * We now use the config.DevOverrides struct for the specific overrides, splitting overrides from the configuration struct they were previously stuffed in * We now avoid changing the config struct as much in auth0 NewClient
To be used in the next commit...
* DisableFlagParsing has been improved, we now use cobraext.StripFlagset to split rpk specific flags from unknown plugin flags. We now parse the rpk specific flags into our master config.Params struct, which allows us to use it as we expect. * We wire config.Params into every plugin aspect * We remove the now-unneeded cloudcfg package (which has been replaced by the config package / config.Params) * We fix everything given prior changes
* Tightly maps xflags with which yaml field they are expected to set * Adds functions to return pairs of xflags to their yaml modifiers * Ensures that using some value as we expect sets the field we expect
* removes duplicate-to, which is a bit confusing * supports -X flags and yaml paths for autocompletion, defaulting to the yaml path * set_test.go has been removed since it is now completely tested in config's params_test.go
It's difficult to reason about whether to error or not if one of the redpanda.yaml or rpk.yaml fails to be written. Keeping tuners strictly in redpanda.yaml isn't too bad of a papercut, and this allows us to wait some time to figure out what we want to do in the future. In preparation of the future, though, SetMode has been moved to the Config struct. As well, we now autocomplete the two modes supported.
"Materialized" clasically refers to a DB that is actually backed to a file. "Virtual" refers to something kept entirely in memory. We swap to Virtual to avoid future confusion for anybody more familiar with databases.
This is similar to the client timeout / might be the same, so this can potentially be removed. Used at the moment while the cloudapi is on the fritz.
FromCloud will be used when logging in, see next commits.
For reuse in the next commit, making login a bit seamless.
The proper terminology for SASL per the original RFC4422 is "mechanism". Type is shorter but technically not correct, and our flags have always referred to this as "--sasl-mechanism". We switch the new -X flag to be "-X sasl.mechanism" to closely match the old flag, and we now encode the SASL field with "mechanism". We still support decoding "type" but we will rewrite it as "mechanism" when we encode. This does also affect redpanda.yaml, meaning if a person modifies the rpk section in redpanda.yaml, then older rpk's will no longer be able to read their yaml. We historically do not support forward compatibility like that (requiring old versions to work on new), and rpk will be modifying rpk.yaml and not redpanda.yaml without some deliberate effort, so this is perceived as low risk.
This at least ensures that if a person adds a field anywhere in rpk.yaml, we force a version bump (in review). This also prevents problems that we ran into with redpanda.yaml in the past, where a field would be stripped by rpk because rpk's struct wasn't updated or an old rpk was running. In redpanda.yaml, this was "fixed" by adding `Other` basically everywhere. In rpk.yaml, we can do strict version checking: new rpk's will support old rpk.yaml versions, old rpk's will fail because of the version check up front, and this test forces us to bump the version whenever appropriate.
r-vasquez
previously approved these changes
May 25, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, definitely introduces a lot of good stuff 🙌
r-vasquez
approved these changes
May 25, 2023
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.
Backports Required
Release Notes
Features
rpk profile
to manage multiple rpk configuration profilesrpk cloud auth
to manage multiple rpk cloud authsrpk cloud login
Improvements
ca_file
rather thantruststore_file
, andmechanism
rather thantype
for the SASL struct. This means that once you write a config with the new rpk (v23.2.x+), the config will not be able to be read with an older rpk.