CORE-7343 License: add fallback license env var #23583
Merged
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
pgellert
requested review from
oleiman and
michael-redpanda
and removed request for
a team
|
/dt |
oleiman
reviewed
Oct 1, 2024
src/v/cluster/feature_manager.cc
Outdated
| @@ -309,17 +309,36 @@ void feature_manager::verify_enterprise_license() { | |||
| } | |||
|
|
|||
| const auto& license = _feature_table.local().get_license(); | |||
| auto license_missing_or_expired = !license || license->is_expired(); | |||
| std::optional<security::license> fallback_license = std::nullopt; | |||
| auto fallback_license_str = std::getenv("RP_FALLBACK_ENTERPRISE_LICENSE"); | |||
There was a problem hiding this comment.
I don't think there's tons of precedent for this, but maybe
Suggested change
| auto fallback_license_str = std::getenv("RP_FALLBACK_ENTERPRISE_LICENSE"); | |
| auto fallback_license_str = std::getenv("REDPANDA_FALLBACK_ENTERPRISE_LICENSE"); |
There was a problem hiding this comment.
The prior art of existing env vars:
- Redpanda:
RP_SI_CREDS_API_ADDRESS (undocumented)
RP_BOOTSTRAP_USER (only documented Redpanda env var)
RP_FIXTURE_ENV (testing)
REDPANDA_ENVIRONMENT (undocumented, set by operator to "kubernetes", used for phone-home data)
__REDPANDA_SKIP_IAM_TOKEN (testing)
__REDPANDA_LICENSE_CHECK_INTERVAL_SEC (testing)
__REDPANDA_TOPIC_REC_DL_CHECK_MILLIS (testing)
__REDPANDA_TEST_FEATURES (testing)
__REDPANDA_TEST_FEATURE_NO_AUTO_ACTIVATE_BRAVO (testing)
__REDPANDA_LATEST_LOGICAL_VERSION (testing)
__REDPANDA_EARLIEST_LOGICAL_VERSION (testing)
__REDPANDA_TEST_DISK_SIZE (testing)
- Rpk uses
REDPANDA_prefix - Wasm transforms use the
REDPANDA_prefix
So, there is no clear consistency so far, but I think it makes sense to prefer the REDPANDA_ prefix for this and env vars going forward. Some env vars also use the __ prefix, but that seems to be the pattern for test-only variables, so I think we don't want to add that for this variable.
I've changed this to REDPANDA_FALLBACK_ENTERPRISE_LICENSE now.
There was a problem hiding this comment.
oh yeah, missed RP_BOOTSTRAP_USER. The __ prefix is definitely meant for "hidden" knobs, so no worries there. I don't think it's worth belaboring too much, but we might want to ask product.
pgellert
force-pushed
the
license/escape-hatch
branch
from
62cd9b9 to
fb8fc5f
Compare
|
michael-redpanda
previously approved these changes
Oct 2, 2024
|
new failures in https://buildkite.com/redpanda/redpanda/builds/55642#01924cfd-80f8-4475-83b8-9c35ab4bd3be: |
oleiman
previously approved these changes
Oct 2, 2024
...instead of `const ss:string&` to allow passing in the contents of an environment variable stored as a `char*`.
This is to make it easier to reuse the assertion and its message outside of `redpanda.install_license`. I am going to reuse it in the next commit.
This adds an environment variable that can be used to insert an enterprise license on startup into a non-compliant cluster when it is failing the license enforcement check. This should be an undocumented escape hatch we can use if we ever need to inject a license into this check without being able to add it into the controller log via `rpk cluster license set` (eg. if we can't downgrade the broker to a previous version).
pgellert
dismissed stale reviews from oleiman and michael-redpanda
via
0847ba1
pgellert
force-pushed
the
license/escape-hatch
branch
from
fb8fc5f to
0847ba1
Compare
|
Force-push: rebased to dev now that the base PR is merged |
michael-redpanda
approved these changes
Oct 4, 2024
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 is part of a series of changes to verify that clusters have a valid enterprise license during major version upgrades if they use enterprise features.
This adds an environment variable that can be used to insert an enterprise license on startup into a non-compliant cluster when it is failing the license enforcement check. This should be an undocumented escape hatch we can use if we ever need to inject a license into this check if we cannot add it into the controller log via
rpk cluster license set(eg. if we cannot downgrade the broker to a previous version for any reason).Fixes https://redpandadata.atlassian.net/browse/CORE-7343
Backports Required
Release Notes