Additive updates to services when discovery enabled #2318
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
ashutosh-narkar
changed the title
tsandall
reviewed
Apr 24, 2020
docs/content/management.md
Outdated
| The discovery feature cannot be used to dynamically modify `labels` and `discovery`. This means that these configuration settings should be included | ||
| in the bootup configuration file provided to OPA. However, it can be used to modify `services` with the exception of the service used to download the discovery bundle. |
There was a problem hiding this comment.
Would be good to include a note here about why these settings cannot be modified and what happens if you try to. Something like:
In practice, discovery services do not change frequently. These configuration sections are treated as
immutable to avoid accidental configuration errors rendering OPA unable to discover new configuration.
If the discovered configuration changes the `discovery` or `labels` sections,
those changes are ignored. If the discovered configuration changes the discovery service,
an error will be logged.
This makes me wonder why we're erroring on disco service changes but not disco or label section changes.
| // clients can be specified either as an array or as a map. Some systems (e.g., | ||
| // Helm) do not have proper support for configuration values nested under | ||
| // arrays, so just support both here. | ||
| func ParseServicesConfig(raw json.RawMessage) (map[string]rest.Client, error) { |
There was a problem hiding this comment.
Just a side note, but I imagine us having to change this API slightly in the future when we introduce support for other bundle download protocols like Git. In that case, the service will no longer be a rest.Client. No changes required at this time since this is just an internal API.
|
@ashutosh-narkar if you update the docs, we can merge this. |
ashutosh-narkar
force-pushed
the
disco-updates
branch
from
56900e0 to
24762b1
Compare
|
Doc updated. |
Earlier with discovery enabled, there was no protection against accidental changes to the discovery service. This change prevents the discovery service from being modified by checking it's config in the service bundle. Fixes open-policy-agent#2058 Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
ashutosh-narkar
force-pushed
the
disco-updates
branch
from
24762b1 to
5261011
Compare
patrick-east
approved these changes
Apr 29, 2020
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.
Earlier with discovery enabled, there was no protection against accidental
changes to the discovery service. This change prevents the discovery service
from being modified by checking it's config in the service bundle.
Fixes #2058