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
plugins/discovery: ensure discovery doesn't erase its own config #6070
plugins/discovery: ensure discovery doesn't erase its own config #6070
Conversation
aeb98ee
to
02aff38
Compare
fixes #6046 |
@blacksails the changes look fine. Can you please fix the broken tests. Thanks. |
(I think this is all you need) diff --git a/sdk/opa_test.go b/sdk/opa_test.go
index 5fbe773c0..63e89bc76 100644
--- a/sdk/opa_test.go
+++ b/sdk/opa_test.go
@@ -161,7 +161,8 @@ func TestHookOnConfigDiscovery(t *testing.T) {
"foo": "baz",
"fox": "quz",
},
- Plugins: map[string]json.RawMessage{"test_plugin": json.RawMessage("{}")},
+ Plugins: map[string]json.RawMessage{"test_plugin": json.RawMessage("{}")},
+ Discovery: json.RawMessage(`{"service":"disco", "resource": "disco.tar.gz"}`),
}
act := th1.c // doesn't matter which hook, they only mutate the config via its pointer
if diff := cmp.Diff(exp, act, cmpopts.IgnoreFields(config.Config{}, "DefaultDecision", "DefaultAuthorizationDecision")); diff != "" { |
When the discovery plugin receives a a discovery bundle which omits the discovery configuration, it deletes its own configuration from the manager. In turn this means that `GET /v1/config` doesn't show the configuration of the discovery plugin. This change ensures that the plugin will never overwrite the discovery configuration on the manager. Signed-off-by: Benjamin Nørgaard <mail@blacksails.dev>
19f39c8
to
8f55f40
Compare
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Sorry about the delay. Fixed test and rebased. |
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.
This looks good to me 🙂. I'll let Ash have a final check before we merge. Thanks for working on it!
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.
LGTM
When the discovery plugin receives a a discovery bundle which omits the
discovery configuration, it deletes its own configuration from the
manager. In turn this means that
GET /v1/config
doesn't show theconfiguration of the discovery plugin.
This change ensures that the plugin will never overwrite the discovery
configuration on the manager.
Signed-off-by: Benjamin Nørgaard mail@blacksails.dev