Skip to content
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.

Sign up for GitHub

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

Jump to bottom

plugins/discovery: ensure discovery doesn't erase its own config #6070

Merged
merged 1 commit into from
Jul 6, 2023
Merged

plugins/discovery: ensure discovery doesn't erase its own config #6070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions plugins/discovery/discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ func (c *Discovery) processBundle(ctx context.Context, b *bundleApi.Bundle) (*pl
// Note: We don't currently support changes to the discovery
// configuration. These changes are risky because errors would be
// unrecoverable (without keeping track of changes and rolling back...)
config.Discovery = c.manager.Config.Discovery

// check for updates to the discovery service
opts := cfg.ServiceOptions{
Expand Down
13 changes: 10 additions & 3 deletions plugins/discovery/discovery_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ func TestProcessBundleWithActiveConfig(t *testing.T) {
}
},
"default_authorization_decision": "baz/qux",
"default_decision": "bar/baz"}`, version.Version)
"default_decision": "bar/baz",
"discovery": {"name": "config"}
}`, version.Version)

var expected map[string]interface{}
if err := util.Unmarshal([]byte(expectedConfig), &expected); err != nil {
Expand Down Expand Up @@ -340,7 +342,9 @@ func TestProcessBundleWithActiveConfig(t *testing.T) {
}
},
"default_authorization_decision": "/system/authz/allow",
"default_decision": "/system/main"}`, version.Version)
"default_decision": "/system/main",
"discovery": {"name": "config"}
}`, version.Version)

var expected2 map[string]interface{}
if err := util.Unmarshal([]byte(expectedConfig2), &expected2); err != nil {
Expand Down Expand Up @@ -1348,7 +1352,7 @@ func TestReconfigureWithUpdates(t *testing.T) {
t.Fatalf("Unexpected error %v", err)
}

// check that not setting persistence_directory doesn't remove boot config
// check that omitting persistence_directory or discovery doesn't remove boot config
updatedBundle = makeDataBundle(13, `
{
"config": {}
Expand All @@ -1363,6 +1367,9 @@ func TestReconfigureWithUpdates(t *testing.T) {
if manager.Config.PersistenceDirectory == nil {
t.Fatal("Erased persistence directory configuration")
}
if manager.Config.Discovery == nil {
t.Fatal("Erased discovery plugin configuration")
}

// update persistence directory
updatedBundle = makeDataBundle(14, `
Expand Down
3 changes: 2 additions & 1 deletion sdk/opa_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down