From 2a8c79eacee1a9856a469e01c2804db2d77a20e1 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:25:22 +1000 Subject: [PATCH 01/10] Move RemoveSecret from apiserver/facades/agent/secretsmanager/ to apiserver/common/secrets/; --- .../common/secrets/mocks/authorizer_mock.go | 190 ++++++++++++++++++ .../secrets/mocks/commonsecrets_mock.go | 60 +++++- .../common/secrets/mocks/provider_mock.go | 85 +++++++- apiserver/common/secrets/package_test.go | 5 +- apiserver/common/secrets/secrets.go | 102 ++++++++++ apiserver/common/secrets/secrets_test.go | 143 +++++++++++++ apiserver/common/secrets/state.go | 6 + .../agent/secretsmanager/package_test.go | 1 + .../facades/agent/secretsmanager/register.go | 32 +-- .../facades/agent/secretsmanager/secrets.go | 76 +------ .../agent/secretsmanager/secrets_test.go | 7 +- 11 files changed, 617 insertions(+), 90 deletions(-) create mode 100644 apiserver/common/secrets/mocks/authorizer_mock.go diff --git a/apiserver/common/secrets/mocks/authorizer_mock.go b/apiserver/common/secrets/mocks/authorizer_mock.go new file mode 100644 index 00000000000..a2c34c0c6e3 --- /dev/null +++ b/apiserver/common/secrets/mocks/authorizer_mock.go @@ -0,0 +1,190 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/apiserver/facade (interfaces: Authorizer) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + permission "github.com/juju/juju/core/permission" + names "github.com/juju/names/v4" + gomock "go.uber.org/mock/gomock" +) + +// MockAuthorizer is a mock of Authorizer interface. +type MockAuthorizer struct { + ctrl *gomock.Controller + recorder *MockAuthorizerMockRecorder +} + +// MockAuthorizerMockRecorder is the mock recorder for MockAuthorizer. +type MockAuthorizerMockRecorder struct { + mock *MockAuthorizer +} + +// NewMockAuthorizer creates a new mock instance. +func NewMockAuthorizer(ctrl *gomock.Controller) *MockAuthorizer { + mock := &MockAuthorizer{ctrl: ctrl} + mock.recorder = &MockAuthorizerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAuthorizer) EXPECT() *MockAuthorizerMockRecorder { + return m.recorder +} + +// AuthApplicationAgent mocks base method. +func (m *MockAuthorizer) AuthApplicationAgent() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthApplicationAgent") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthApplicationAgent indicates an expected call of AuthApplicationAgent. +func (mr *MockAuthorizerMockRecorder) AuthApplicationAgent() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthApplicationAgent", reflect.TypeOf((*MockAuthorizer)(nil).AuthApplicationAgent)) +} + +// AuthClient mocks base method. +func (m *MockAuthorizer) AuthClient() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthClient") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthClient indicates an expected call of AuthClient. +func (mr *MockAuthorizerMockRecorder) AuthClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthClient", reflect.TypeOf((*MockAuthorizer)(nil).AuthClient)) +} + +// AuthController mocks base method. +func (m *MockAuthorizer) AuthController() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthController") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthController indicates an expected call of AuthController. +func (mr *MockAuthorizerMockRecorder) AuthController() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthController", reflect.TypeOf((*MockAuthorizer)(nil).AuthController)) +} + +// AuthMachineAgent mocks base method. +func (m *MockAuthorizer) AuthMachineAgent() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthMachineAgent") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthMachineAgent indicates an expected call of AuthMachineAgent. +func (mr *MockAuthorizerMockRecorder) AuthMachineAgent() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthMachineAgent", reflect.TypeOf((*MockAuthorizer)(nil).AuthMachineAgent)) +} + +// AuthModelAgent mocks base method. +func (m *MockAuthorizer) AuthModelAgent() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthModelAgent") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthModelAgent indicates an expected call of AuthModelAgent. +func (mr *MockAuthorizerMockRecorder) AuthModelAgent() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthModelAgent", reflect.TypeOf((*MockAuthorizer)(nil).AuthModelAgent)) +} + +// AuthOwner mocks base method. +func (m *MockAuthorizer) AuthOwner(arg0 names.Tag) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthOwner", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthOwner indicates an expected call of AuthOwner. +func (mr *MockAuthorizerMockRecorder) AuthOwner(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthOwner", reflect.TypeOf((*MockAuthorizer)(nil).AuthOwner), arg0) +} + +// AuthUnitAgent mocks base method. +func (m *MockAuthorizer) AuthUnitAgent() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthUnitAgent") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AuthUnitAgent indicates an expected call of AuthUnitAgent. +func (mr *MockAuthorizerMockRecorder) AuthUnitAgent() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthUnitAgent", reflect.TypeOf((*MockAuthorizer)(nil).AuthUnitAgent)) +} + +// ConnectedModel mocks base method. +func (m *MockAuthorizer) ConnectedModel() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectedModel") + ret0, _ := ret[0].(string) + return ret0 +} + +// ConnectedModel indicates an expected call of ConnectedModel. +func (mr *MockAuthorizerMockRecorder) ConnectedModel() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectedModel", reflect.TypeOf((*MockAuthorizer)(nil).ConnectedModel)) +} + +// EntityHasPermission mocks base method. +func (m *MockAuthorizer) EntityHasPermission(arg0 names.Tag, arg1 permission.Access, arg2 names.Tag) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EntityHasPermission", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// EntityHasPermission indicates an expected call of EntityHasPermission. +func (mr *MockAuthorizerMockRecorder) EntityHasPermission(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityHasPermission", reflect.TypeOf((*MockAuthorizer)(nil).EntityHasPermission), arg0, arg1, arg2) +} + +// GetAuthTag mocks base method. +func (m *MockAuthorizer) GetAuthTag() names.Tag { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAuthTag") + ret0, _ := ret[0].(names.Tag) + return ret0 +} + +// GetAuthTag indicates an expected call of GetAuthTag. +func (mr *MockAuthorizerMockRecorder) GetAuthTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthTag", reflect.TypeOf((*MockAuthorizer)(nil).GetAuthTag)) +} + +// HasPermission mocks base method. +func (m *MockAuthorizer) HasPermission(arg0 permission.Access, arg1 names.Tag) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasPermission", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// HasPermission indicates an expected call of HasPermission. +func (mr *MockAuthorizerMockRecorder) HasPermission(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasPermission", reflect.TypeOf((*MockAuthorizer)(nil).HasPermission), arg0, arg1) +} diff --git a/apiserver/common/secrets/mocks/commonsecrets_mock.go b/apiserver/common/secrets/mocks/commonsecrets_mock.go index 21912f9a62b..5d82af94b28 100644 --- a/apiserver/common/secrets/mocks/commonsecrets_mock.go +++ b/apiserver/common/secrets/mocks/commonsecrets_mock.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/common/secrets (interfaces: Model,Credential,SecretsConsumer,SecretsMetaState) +// Source: github.com/juju/juju/apiserver/common/secrets (interfaces: Model,Credential,SecretsConsumer,SecretsMetaState,SecretsRemoveState) // Package mocks is a generated GoMock package. package mocks @@ -295,3 +295,61 @@ func (mr *MockSecretsMetaStateMockRecorder) ListSecrets(arg0 interface{}) *gomoc mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListSecrets", reflect.TypeOf((*MockSecretsMetaState)(nil).ListSecrets), arg0) } + +// MockSecretsRemoveState is a mock of SecretsRemoveState interface. +type MockSecretsRemoveState struct { + ctrl *gomock.Controller + recorder *MockSecretsRemoveStateMockRecorder +} + +// MockSecretsRemoveStateMockRecorder is the mock recorder for MockSecretsRemoveState. +type MockSecretsRemoveStateMockRecorder struct { + mock *MockSecretsRemoveState +} + +// NewMockSecretsRemoveState creates a new mock instance. +func NewMockSecretsRemoveState(ctrl *gomock.Controller) *MockSecretsRemoveState { + mock := &MockSecretsRemoveState{ctrl: ctrl} + mock.recorder = &MockSecretsRemoveStateMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSecretsRemoveState) EXPECT() *MockSecretsRemoveStateMockRecorder { + return m.recorder +} + +// DeleteSecret mocks base method. +func (m *MockSecretsRemoveState) DeleteSecret(arg0 *secrets0.URI, arg1 ...int) ([]secrets0.ValueRef, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteSecret", varargs...) + ret0, _ := ret[0].([]secrets0.ValueRef) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteSecret indicates an expected call of DeleteSecret. +func (mr *MockSecretsRemoveStateMockRecorder) DeleteSecret(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSecret", reflect.TypeOf((*MockSecretsRemoveState)(nil).DeleteSecret), varargs...) +} + +// GetSecret mocks base method. +func (m *MockSecretsRemoveState) GetSecret(arg0 *secrets0.URI) (*secrets0.SecretMetadata, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecret", arg0) + ret0, _ := ret[0].(*secrets0.SecretMetadata) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecret indicates an expected call of GetSecret. +func (mr *MockSecretsRemoveStateMockRecorder) GetSecret(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecret", reflect.TypeOf((*MockSecretsRemoveState)(nil).GetSecret), arg0) +} diff --git a/apiserver/common/secrets/mocks/provider_mock.go b/apiserver/common/secrets/mocks/provider_mock.go index 724522ecf9a..59a819a7bfc 100644 --- a/apiserver/common/secrets/mocks/provider_mock.go +++ b/apiserver/common/secrets/mocks/provider_mock.go @@ -1,12 +1,14 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/secrets/provider (interfaces: SecretBackendProvider) +// Source: github.com/juju/juju/secrets/provider (interfaces: SecretBackendProvider,SecretsBackend) // Package mocks is a generated GoMock package. package mocks import ( + context "context" reflect "reflect" + secrets "github.com/juju/juju/core/secrets" provider "github.com/juju/juju/secrets/provider" names "github.com/juju/names/v4" gomock "go.uber.org/mock/gomock" @@ -120,3 +122,84 @@ func (mr *MockSecretBackendProviderMockRecorder) Type() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Type", reflect.TypeOf((*MockSecretBackendProvider)(nil).Type)) } + +// MockSecretsBackend is a mock of SecretsBackend interface. +type MockSecretsBackend struct { + ctrl *gomock.Controller + recorder *MockSecretsBackendMockRecorder +} + +// MockSecretsBackendMockRecorder is the mock recorder for MockSecretsBackend. +type MockSecretsBackendMockRecorder struct { + mock *MockSecretsBackend +} + +// NewMockSecretsBackend creates a new mock instance. +func NewMockSecretsBackend(ctrl *gomock.Controller) *MockSecretsBackend { + mock := &MockSecretsBackend{ctrl: ctrl} + mock.recorder = &MockSecretsBackendMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSecretsBackend) EXPECT() *MockSecretsBackendMockRecorder { + return m.recorder +} + +// DeleteContent mocks base method. +func (m *MockSecretsBackend) DeleteContent(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteContent", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteContent indicates an expected call of DeleteContent. +func (mr *MockSecretsBackendMockRecorder) DeleteContent(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteContent", reflect.TypeOf((*MockSecretsBackend)(nil).DeleteContent), arg0, arg1) +} + +// GetContent mocks base method. +func (m *MockSecretsBackend) GetContent(arg0 context.Context, arg1 string) (secrets.SecretValue, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetContent", arg0, arg1) + ret0, _ := ret[0].(secrets.SecretValue) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetContent indicates an expected call of GetContent. +func (mr *MockSecretsBackendMockRecorder) GetContent(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetContent", reflect.TypeOf((*MockSecretsBackend)(nil).GetContent), arg0, arg1) +} + +// Ping mocks base method. +func (m *MockSecretsBackend) Ping() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Ping") + ret0, _ := ret[0].(error) + return ret0 +} + +// Ping indicates an expected call of Ping. +func (mr *MockSecretsBackendMockRecorder) Ping() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Ping", reflect.TypeOf((*MockSecretsBackend)(nil).Ping)) +} + +// SaveContent mocks base method. +func (m *MockSecretsBackend) SaveContent(arg0 context.Context, arg1 *secrets.URI, arg2 int, arg3 secrets.SecretValue) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SaveContent", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SaveContent indicates an expected call of SaveContent. +func (mr *MockSecretsBackendMockRecorder) SaveContent(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveContent", reflect.TypeOf((*MockSecretsBackend)(nil).SaveContent), arg0, arg1, arg2, arg3) +} diff --git a/apiserver/common/secrets/package_test.go b/apiserver/common/secrets/package_test.go index 9561288b38b..cb79e863636 100644 --- a/apiserver/common/secrets/package_test.go +++ b/apiserver/common/secrets/package_test.go @@ -9,9 +9,10 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/commonsecrets_mock.go github.com/juju/juju/apiserver/common/secrets Model,Credential,SecretsConsumer,SecretsMetaState +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/commonsecrets_mock.go github.com/juju/juju/apiserver/common/secrets Model,Credential,SecretsConsumer,SecretsMetaState,SecretsRemoveState +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/authorizer_mock.go github.com/juju/juju/apiserver/facade Authorizer //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/leadership_mock.go github.com/juju/juju/core/leadership Checker,Token -//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/provider_mock.go github.com/juju/juju/secrets/provider SecretBackendProvider +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/provider_mock.go github.com/juju/juju/secrets/provider SecretBackendProvider,SecretsBackend //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/state_mock.go github.com/juju/juju/state SecretsStore,SecretBackendsStorage func TestPackage(t *testing.T) { diff --git a/apiserver/common/secrets/secrets.go b/apiserver/common/secrets/secrets.go index f7a635634b3..eeab5d967b4 100644 --- a/apiserver/common/secrets/secrets.go +++ b/apiserver/common/secrets/secrets.go @@ -4,6 +4,7 @@ package secrets import ( + "context" "sort" "github.com/juju/collections/set" @@ -12,9 +13,11 @@ import ( "github.com/juju/names/v4" apiservererrors "github.com/juju/juju/apiserver/errors" + "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/cloud" "github.com/juju/juju/core/leadership" corelogger "github.com/juju/juju/core/logger" + "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" "github.com/juju/juju/environs/cloudspec" @@ -535,3 +538,102 @@ func GetSecretMetadata( } return result, nil } + +// RemoveSecrets removes the specified secrets. +// If removeFromBackend is false, the secrets are only removed from the state and +// the caller must have permission to manage the secret(secret owners remove secrets from the backend on uniter side). +// If removeFromBackend is true, the secrets are removed from the state and +// backend and the caller must have model admin access. +func RemoveSecrets( + secretsState SecretsMetaState, + removeState SecretsRemoveState, + secretsConsumer SecretsConsumer, + adminConfigGetter BackendAdminConfigGetter, + modelTag names.ModelTag, authorizer facade.Authorizer, authTag names.Tag, leadershipChecker leadership.Checker, + args params.DeleteSecretArgs, removeFromBackend bool, +) (params.ErrorResults, error) { + removeSecret := func(uri *coresecrets.URI, revisions ...int) ([]coresecrets.ValueRef, error) { + if _, err := removeState.GetSecret(uri); err != nil { + // Check if the uri exists or not. + return nil, errors.Trace(err) + } + if removeFromBackend { + if err := authorizer.HasPermission(permission.AdminAccess, modelTag); err != nil { + return nil, errors.Trace(err) + } + // Admins can delete any secret in the model. + return removeState.DeleteSecret(uri, revisions...) + } + _, err := CanManage(secretsConsumer, leadershipChecker, authTag, uri) + if err != nil { + return nil, errors.Trace(err) + } + return removeState.DeleteSecret(uri, revisions...) + } + + type deleteInfo struct { + uri *coresecrets.URI + revisions []int + } + toDelete := make([]deleteInfo, len(args.Args)) + for i, arg := range args.Args { + uri, err := coresecrets.ParseURI(arg.URI) + if err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + toDelete[i] = deleteInfo{uri: uri, revisions: arg.Revisions} + } + result := params.ErrorResults{ + Results: make([]params.ErrorResult, len(args.Args)), + } + externalRevisions := make(map[string]provider.SecretRevisions) + for i, d := range toDelete { + external, err := removeSecret(d.uri, d.revisions...) + result.Results[i].Error = apiservererrors.ServerError(err) + if err == nil { + for _, rev := range external { + if _, ok := externalRevisions[rev.BackendID]; !ok { + externalRevisions[rev.BackendID] = provider.SecretRevisions{} + } + externalRevisions[rev.BackendID].Add(d.uri, rev.RevisionID) + } + } + } + logger.Criticalf("externalRevisions: %#v", externalRevisions) + if len(externalRevisions) == 0 { + return result, nil + } + + cfgInfo, err := adminConfigGetter() + if err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + for backendID, r := range externalRevisions { + // TODO: include unitTag in params.DeleteSecretArgs for operator uniters? + // This should be resolved once lp:1991213 and lp:1991854 are fixed. + backendCfg, ok := cfgInfo.Configs[backendID] + logger.Criticalf("backendID %q, cfgInfo.Configs: %#v", backendID, cfgInfo.Configs) + if !ok { + return params.ErrorResults{}, errors.NotFoundf("secret backend %q", backendID) + } + provider, err := GetProvider(backendCfg.BackendType) + if err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + if removeFromBackend { + backend, err := provider.NewBackend(&backendCfg) + if err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + for _, revId := range r.RevisionIDs() { + if err = backend.DeleteContent(context.TODO(), revId); err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + } + } + if err := provider.CleanupSecrets(&backendCfg, authTag, r); err != nil { + return params.ErrorResults{}, errors.Trace(err) + } + } + return result, nil +} diff --git a/apiserver/common/secrets/secrets_test.go b/apiserver/common/secrets/secrets_test.go index cdb51c9fec6..ed9e414e4bc 100644 --- a/apiserver/common/secrets/secrets_test.go +++ b/apiserver/common/secrets/secrets_test.go @@ -17,6 +17,7 @@ import ( "github.com/juju/juju/apiserver/common/secrets/mocks" "github.com/juju/juju/cloud" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" "github.com/juju/juju/secrets/provider" @@ -722,3 +723,145 @@ func (s *secretsSuite) TestGetSecretMetadata(c *gc.C) { }}, }) } + +func (s *secretsSuite) TestRemoveSecretsForSecretOwners(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + expectURI := *uri + secretsMetaState := mocks.NewMockSecretsMetaState(ctrl) + removeState := mocks.NewMockSecretsRemoveState(ctrl) + secretsConsumer := mocks.NewMockSecretsConsumer(ctrl) + authorizer := mocks.NewMockAuthorizer(ctrl) + leadershipChecker := mocks.NewMockChecker(ctrl) + mockprovider := mocks.NewMockSecretBackendProvider(ctrl) + s.PatchValue(&secrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return mockprovider, nil }) + + removeState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + removeState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ + BackendID: "backend-id", + RevisionID: "rev-666", + }}, nil) + + secretsConsumer.EXPECT().SecretAccess(uri, names.NewUnitTag("mariadb/0")).Return(coresecrets.RoleManage, nil) + + cfg := &provider.ModelBackendConfig{ + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "fred", + BackendConfig: provider.BackendConfig{ + BackendType: "some-backend", + Config: map[string]interface{}{"foo": "admin"}, + }, + } + mockprovider.EXPECT().CleanupSecrets( + cfg, names.NewUnitTag("mariadb/0"), + provider.SecretRevisions{uri.ID: set.NewStrings("rev-666")}, + ).Return(nil) + + adminConfigGetter := func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "fred", + BackendConfig: provider.BackendConfig{ + BackendType: "some-backend", + Config: map[string]interface{}{"foo": "admin"}, + }, + }, + }, + }, nil + } + + results, err := secrets.RemoveSecrets( + secretsMetaState, removeState, secretsConsumer, adminConfigGetter, coretesting.ModelTag, + authorizer, names.NewUnitTag("mariadb/0"), + leadershipChecker, + params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }, false, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{ + Results: []params.ErrorResult{{}}, + }) +} + +func (s *secretsSuite) TestRemoveSecretsForModelAdmin(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + expectURI := *uri + secretsMetaState := mocks.NewMockSecretsMetaState(ctrl) + removeState := mocks.NewMockSecretsRemoveState(ctrl) + secretsConsumer := mocks.NewMockSecretsConsumer(ctrl) + authorizer := mocks.NewMockAuthorizer(ctrl) + leadershipChecker := mocks.NewMockChecker(ctrl) + mockprovider := mocks.NewMockSecretBackendProvider(ctrl) + backend := mocks.NewMockSecretsBackend(ctrl) + s.PatchValue(&secrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return mockprovider, nil }) + + authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) + removeState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + removeState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ + BackendID: "backend-id", + RevisionID: "rev-666", + }}, nil) + + cfg := &provider.ModelBackendConfig{ + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "fred", + BackendConfig: provider.BackendConfig{ + BackendType: "some-backend", + Config: map[string]interface{}{"foo": "admin"}, + }, + } + mockprovider.EXPECT().NewBackend(cfg).Return(backend, nil) + backend.EXPECT().DeleteContent(gomock.Any(), "rev-666").Return(nil) + mockprovider.EXPECT().CleanupSecrets( + cfg, names.NewUserTag("foo"), + provider.SecretRevisions{uri.ID: set.NewStrings("rev-666")}, + ).Return(nil) + + adminConfigGetter := func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "fred", + BackendConfig: provider.BackendConfig{ + BackendType: "some-backend", + Config: map[string]interface{}{"foo": "admin"}, + }, + }, + }, + }, nil + } + + results, err := secrets.RemoveSecrets( + secretsMetaState, removeState, secretsConsumer, adminConfigGetter, coretesting.ModelTag, + authorizer, names.NewUserTag("foo"), + leadershipChecker, + params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }, true, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{ + Results: []params.ErrorResult{{}}, + }) +} diff --git a/apiserver/common/secrets/state.go b/apiserver/common/secrets/state.go index 0fb8445e86b..64036497bb5 100644 --- a/apiserver/common/secrets/state.go +++ b/apiserver/common/secrets/state.go @@ -52,6 +52,12 @@ type SecretsMetaState interface { ListSecretRevisions(uri *secrets.URI) ([]*secrets.SecretRevisionMetadata, error) } +// SecretsRemoveState instances provide secret removal apis. +type SecretsRemoveState interface { + DeleteSecret(*secrets.URI, ...int) ([]secrets.ValueRef, error) + GetSecret(*secrets.URI) (*secrets.SecretMetadata, error) +} + // Credential represents a cloud credential. type Credential interface { AuthType() string diff --git a/apiserver/facades/agent/secretsmanager/package_test.go b/apiserver/facades/agent/secretsmanager/package_test.go index 9aa1cf6d30d..79186ce2e6e 100644 --- a/apiserver/facades/agent/secretsmanager/package_test.go +++ b/apiserver/facades/agent/secretsmanager/package_test.go @@ -52,6 +52,7 @@ func NewTestAPI( return &SecretsManagerAPI{ authTag: authTag, + authorizer: authorizer, resources: resources, leadershipChecker: leadership, secretsState: secretsState, diff --git a/apiserver/facades/agent/secretsmanager/register.go b/apiserver/facades/agent/secretsmanager/register.go index 9f6bca1e29d..93724e61c9f 100644 --- a/apiserver/facades/agent/secretsmanager/register.go +++ b/apiserver/facades/agent/secretsmanager/register.go @@ -14,8 +14,9 @@ import ( "github.com/juju/juju/api/controller/crossmodelsecrets" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/common/secrets" - apiservererrors "github.com/juju/juju/apiserver/errors" + // apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" + "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" "github.com/juju/juju/secrets/provider" @@ -43,34 +44,34 @@ func NewSecretManagerAPIV1(context facade.Context) (*SecretsManagerAPIV1, error) return &SecretsManagerAPIV1{SecretsManagerAPI: api}, nil } +func checkPermission(context facade.Context) error { + if context.Auth().AuthUnitAgent() || context.Auth().AuthApplicationAgent() { + return nil + } + return context.Auth().HasPermission(permission.WriteAccess, names.NewModelTag(context.State().ModelUUID())) +} + // NewSecretManagerAPI creates a SecretsManagerAPI. func NewSecretManagerAPI(context facade.Context) (*SecretsManagerAPI, error) { - if !context.Auth().AuthUnitAgent() && !context.Auth().AuthApplicationAgent() { - return nil, apiservererrors.ErrPerm + if err := checkPermission(context); err != nil { + return nil, errors.Trace(err) } + model, err := context.State().Model() + if err != nil { + return nil, errors.Trace(err) + } + leadershipChecker, err := context.LeadershipChecker() if err != nil { return nil, errors.Trace(err) } secretBackendConfigGetter := func(backendIDs []string, wantAll bool) (*provider.ModelBackendConfigInfo, error) { - model, err := context.State().Model() - if err != nil { - return nil, errors.Trace(err) - } return secrets.BackendConfigInfo(secrets.SecretsModel(model), backendIDs, wantAll, context.Auth().GetAuthTag(), leadershipChecker) } secretBackendAdminConfigGetter := func() (*provider.ModelBackendConfigInfo, error) { - model, err := context.State().Model() - if err != nil { - return nil, errors.Trace(err) - } return secrets.AdminBackendConfigInfo(secrets.SecretsModel(model)) } secretBackendDrainConfigGetter := func(backendID string) (*provider.ModelBackendConfigInfo, error) { - model, err := context.State().Model() - if err != nil { - return nil, errors.Trace(err) - } return secrets.DrainBackendConfigInfo(backendID, secrets.SecretsModel(model), context.Auth().GetAuthTag(), leadershipChecker) } controllerAPI := common.NewStateControllerConfig(context.State()) @@ -98,6 +99,7 @@ func NewSecretManagerAPI(context facade.Context) (*SecretsManagerAPI, error) { } return &SecretsManagerAPI{ + authorizer: context.Auth(), authTag: context.Auth().GetAuthTag(), leadershipChecker: leadershipChecker, secretsState: state.NewSecrets(context.State()), diff --git a/apiserver/facades/agent/secretsmanager/secrets.go b/apiserver/facades/agent/secretsmanager/secrets.go index 003dc2d9321..07b2fe55ab4 100644 --- a/apiserver/facades/agent/secretsmanager/secrets.go +++ b/apiserver/facades/agent/secretsmanager/secrets.go @@ -28,11 +28,6 @@ import ( var logger = loggo.GetLoggerWithLabels("juju.apiserver.secretsmanager", corelogger.SECRETS) -// For testing. -var ( - GetProvider = secretsprovider.Provider -) - // CrossModelSecretsClient gets secret content from a cross model controller. type CrossModelSecretsClient interface { GetRemoteSecretContentInfo(uri *coresecrets.URI, revision int, refresh, peek bool, appToken string, unitId int, macs macaroon.Slice) (*secrets.ContentParams, *secretsprovider.ModelBackendConfig, int, bool, error) @@ -41,6 +36,7 @@ type CrossModelSecretsClient interface { // SecretsManagerAPI is the implementation for the SecretsManager facade. type SecretsManagerAPI struct { + authorizer facade.Authorizer leadershipChecker leadership.Checker secretsState SecretsState resources facade.Resources @@ -358,71 +354,11 @@ func (s *SecretsManagerAPI) updateSecret(arg params.UpdateSecretArg) error { // RemoveSecrets removes the specified secrets. func (s *SecretsManagerAPI) RemoveSecrets(args params.DeleteSecretArgs) (params.ErrorResults, error) { - type deleteInfo struct { - uri *coresecrets.URI - revisions []int - } - toDelete := make([]deleteInfo, len(args.Args)) - for i, arg := range args.Args { - uri, err := coresecrets.ParseURI(arg.URI) - if err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - toDelete[i] = deleteInfo{uri: uri, revisions: arg.Revisions} - } - result := params.ErrorResults{ - Results: make([]params.ErrorResult, len(args.Args)), - } - externalRevisions := make(map[string]secretsprovider.SecretRevisions) - for i, d := range toDelete { - external, err := s.removeSecret(d.uri, d.revisions...) - result.Results[i].Error = apiservererrors.ServerError(err) - if err == nil { - for _, rev := range external { - if _, ok := externalRevisions[rev.BackendID]; !ok { - externalRevisions[rev.BackendID] = secretsprovider.SecretRevisions{} - } - externalRevisions[rev.BackendID].Add(d.uri, rev.RevisionID) - } - } - } - if len(externalRevisions) == 0 { - return result, nil - } - - cfgInfo, err := s.adminConfigGetter() - if err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - for backendID, r := range externalRevisions { - // TODO: include unitTag in params.DeleteSecretArgs for operator uniters? - // This should be resolved once lp:1991213 and lp:1991854 are fixed. - backendCfg, ok := cfgInfo.Configs[backendID] - if !ok { - return params.ErrorResults{}, errors.NotFoundf("secret backend %q", backendID) - } - provider, err := GetProvider(backendCfg.BackendType) - if err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - if err := provider.CleanupSecrets(&backendCfg, s.authTag, r); err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - } - return result, nil -} - -func (s *SecretsManagerAPI) removeSecret(uri *coresecrets.URI, revisions ...int) ([]coresecrets.ValueRef, error) { - if _, err := s.secretsState.GetSecret(uri); err != nil { - // Check if the uri exists or not. - return nil, errors.Trace(err) - } - - _, err := s.canManage(uri) - if err != nil { - return nil, errors.Trace(err) - } - return s.secretsState.DeleteSecret(uri, revisions...) + return commonsecrets.RemoveSecrets( + s.secretsState, s.secretsState, s.secretsConsumer, s.adminConfigGetter, + names.NewModelTag(s.modelUUID), s.authorizer, s.authTag, s.leadershipChecker, + args, false, + ) } // GetConsumerSecretsRevisionInfo returns the latest secret revisions for the specified secrets. diff --git a/apiserver/facades/agent/secretsmanager/secrets_test.go b/apiserver/facades/agent/secretsmanager/secrets_test.go index 3db7b2feef4..5bcd15f8777 100644 --- a/apiserver/facades/agent/secretsmanager/secrets_test.go +++ b/apiserver/facades/agent/secretsmanager/secrets_test.go @@ -19,9 +19,12 @@ import ( "gopkg.in/macaroon.v2" apitesting "github.com/juju/juju/api/testing" + commonsecrets "github.com/juju/juju/apiserver/common/secrets" + apiservererrors "github.com/juju/juju/apiserver/errors" facademocks "github.com/juju/juju/apiserver/facade/mocks" "github.com/juju/juju/apiserver/facades/agent/secretsmanager" "github.com/juju/juju/apiserver/facades/agent/secretsmanager/mocks" + "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" corewatcher "github.com/juju/juju/core/watcher" "github.com/juju/juju/rpc/params" @@ -78,7 +81,7 @@ func (s *SecretsManagerSuite) setup(c *gc.C) *gomock.Controller { s.secretsTriggerWatcher = mocks.NewMockSecretsTriggerWatcher(ctrl) s.expectAuthUnitAgent() - s.PatchValue(&secretsmanager.GetProvider, func(string) (provider.SecretBackendProvider, error) { return s.provider, nil }) + s.PatchValue(&commonsecrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return s.provider, nil }) s.clock = testclock.NewClock(time.Now()) @@ -465,6 +468,7 @@ func (s *SecretsManagerSuite) TestRemoveSecrets(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(apiservererrors.ErrPerm) s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ BackendID: "backend-id", @@ -504,6 +508,7 @@ func (s *SecretsManagerSuite) TestRemoveSecretRevision(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(apiservererrors.ErrPerm) s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return(nil, nil) s.leadership.EXPECT().LeadershipCheck("mariadb", "mariadb/0").Return(s.token) From 324540f0a06d9b683d5b451f4798f79c78fc4d08 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:28:09 +1000 Subject: [PATCH 02/10] Implement apiserver facade methods: UpdateSecrets and RemoveSecrets; --- .../facades/agent/secretsmanager/register.go | 1 - .../client/secrets/mocks/secretsbackend.go | 113 ++- .../client/secrets/mocks/secretsstate.go | 66 ++ .../facades/client/secrets/package_test.go | 6 +- apiserver/facades/client/secrets/register.go | 1 + apiserver/facades/client/secrets/secrets.go | 131 +++- .../facades/client/secrets/secrets_test.go | 314 +++++++- apiserver/facades/client/secrets/state.go | 6 + apiserver/facades/schema.json | 708 +++++++++++++++++- 9 files changed, 1309 insertions(+), 37 deletions(-) diff --git a/apiserver/facades/agent/secretsmanager/register.go b/apiserver/facades/agent/secretsmanager/register.go index 93724e61c9f..93ab59c62b0 100644 --- a/apiserver/facades/agent/secretsmanager/register.go +++ b/apiserver/facades/agent/secretsmanager/register.go @@ -14,7 +14,6 @@ import ( "github.com/juju/juju/api/controller/crossmodelsecrets" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/common/secrets" - // apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" diff --git a/apiserver/facades/client/secrets/mocks/secretsbackend.go b/apiserver/facades/client/secrets/mocks/secretsbackend.go index da8a18cc77c..bebc8545c8b 100644 --- a/apiserver/facades/client/secrets/mocks/secretsbackend.go +++ b/apiserver/facades/client/secrets/mocks/secretsbackend.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/secrets/provider (interfaces: SecretsBackend) +// Source: github.com/juju/juju/secrets/provider (interfaces: SecretsBackend,SecretBackendProvider) // Package mocks is a generated GoMock package. package mocks @@ -9,6 +9,8 @@ import ( reflect "reflect" secrets "github.com/juju/juju/core/secrets" + provider "github.com/juju/juju/secrets/provider" + names "github.com/juju/names/v4" gomock "go.uber.org/mock/gomock" ) @@ -92,3 +94,112 @@ func (mr *MockSecretsBackendMockRecorder) SaveContent(arg0, arg1, arg2, arg3 int mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveContent", reflect.TypeOf((*MockSecretsBackend)(nil).SaveContent), arg0, arg1, arg2, arg3) } + +// MockSecretBackendProvider is a mock of SecretBackendProvider interface. +type MockSecretBackendProvider struct { + ctrl *gomock.Controller + recorder *MockSecretBackendProviderMockRecorder +} + +// MockSecretBackendProviderMockRecorder is the mock recorder for MockSecretBackendProvider. +type MockSecretBackendProviderMockRecorder struct { + mock *MockSecretBackendProvider +} + +// NewMockSecretBackendProvider creates a new mock instance. +func NewMockSecretBackendProvider(ctrl *gomock.Controller) *MockSecretBackendProvider { + mock := &MockSecretBackendProvider{ctrl: ctrl} + mock.recorder = &MockSecretBackendProviderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSecretBackendProvider) EXPECT() *MockSecretBackendProviderMockRecorder { + return m.recorder +} + +// CleanupModel mocks base method. +func (m *MockSecretBackendProvider) CleanupModel(arg0 *provider.ModelBackendConfig) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CleanupModel", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// CleanupModel indicates an expected call of CleanupModel. +func (mr *MockSecretBackendProviderMockRecorder) CleanupModel(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanupModel", reflect.TypeOf((*MockSecretBackendProvider)(nil).CleanupModel), arg0) +} + +// CleanupSecrets mocks base method. +func (m *MockSecretBackendProvider) CleanupSecrets(arg0 *provider.ModelBackendConfig, arg1 names.Tag, arg2 provider.SecretRevisions) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CleanupSecrets", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// CleanupSecrets indicates an expected call of CleanupSecrets. +func (mr *MockSecretBackendProviderMockRecorder) CleanupSecrets(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanupSecrets", reflect.TypeOf((*MockSecretBackendProvider)(nil).CleanupSecrets), arg0, arg1, arg2) +} + +// Initialise mocks base method. +func (m *MockSecretBackendProvider) Initialise(arg0 *provider.ModelBackendConfig) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Initialise", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// Initialise indicates an expected call of Initialise. +func (mr *MockSecretBackendProviderMockRecorder) Initialise(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Initialise", reflect.TypeOf((*MockSecretBackendProvider)(nil).Initialise), arg0) +} + +// NewBackend mocks base method. +func (m *MockSecretBackendProvider) NewBackend(arg0 *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewBackend", arg0) + ret0, _ := ret[0].(provider.SecretsBackend) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// NewBackend indicates an expected call of NewBackend. +func (mr *MockSecretBackendProviderMockRecorder) NewBackend(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewBackend", reflect.TypeOf((*MockSecretBackendProvider)(nil).NewBackend), arg0) +} + +// RestrictedConfig mocks base method. +func (m *MockSecretBackendProvider) RestrictedConfig(arg0 *provider.ModelBackendConfig, arg1 bool, arg2 names.Tag, arg3, arg4 provider.SecretRevisions) (*provider.BackendConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RestrictedConfig", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(*provider.BackendConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RestrictedConfig indicates an expected call of RestrictedConfig. +func (mr *MockSecretBackendProviderMockRecorder) RestrictedConfig(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestrictedConfig", reflect.TypeOf((*MockSecretBackendProvider)(nil).RestrictedConfig), arg0, arg1, arg2, arg3, arg4) +} + +// Type mocks base method. +func (m *MockSecretBackendProvider) Type() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Type") + ret0, _ := ret[0].(string) + return ret0 +} + +// Type indicates an expected call of Type. +func (mr *MockSecretBackendProviderMockRecorder) Type() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Type", reflect.TypeOf((*MockSecretBackendProvider)(nil).Type)) +} diff --git a/apiserver/facades/client/secrets/mocks/secretsstate.go b/apiserver/facades/client/secrets/mocks/secretsstate.go index 92dfa9fa426..fb24a920639 100644 --- a/apiserver/facades/client/secrets/mocks/secretsstate.go +++ b/apiserver/facades/client/secrets/mocks/secretsstate.go @@ -9,6 +9,7 @@ import ( secrets "github.com/juju/juju/core/secrets" state "github.com/juju/juju/state" + names "github.com/juju/names/v4" gomock "go.uber.org/mock/gomock" ) @@ -50,6 +51,41 @@ func (mr *MockSecretsStateMockRecorder) CreateSecret(arg0, arg1 interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateSecret", reflect.TypeOf((*MockSecretsState)(nil).CreateSecret), arg0, arg1) } +// DeleteSecret mocks base method. +func (m *MockSecretsState) DeleteSecret(arg0 *secrets.URI, arg1 ...int) ([]secrets.ValueRef, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteSecret", varargs...) + ret0, _ := ret[0].([]secrets.ValueRef) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteSecret indicates an expected call of DeleteSecret. +func (mr *MockSecretsStateMockRecorder) DeleteSecret(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSecret", reflect.TypeOf((*MockSecretsState)(nil).DeleteSecret), varargs...) +} + +// GetSecret mocks base method. +func (m *MockSecretsState) GetSecret(arg0 *secrets.URI) (*secrets.SecretMetadata, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecret", arg0) + ret0, _ := ret[0].(*secrets.SecretMetadata) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecret indicates an expected call of GetSecret. +func (mr *MockSecretsStateMockRecorder) GetSecret(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecret", reflect.TypeOf((*MockSecretsState)(nil).GetSecret), arg0) +} + // GetSecretRevision mocks base method. func (m *MockSecretsState) GetSecretRevision(arg0 *secrets.URI, arg1 int) (*secrets.SecretRevisionMetadata, error) { m.ctrl.T.Helper() @@ -111,6 +147,21 @@ func (mr *MockSecretsStateMockRecorder) ListSecrets(arg0 interface{}) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListSecrets", reflect.TypeOf((*MockSecretsState)(nil).ListSecrets), arg0) } +// UpdateSecret mocks base method. +func (m *MockSecretsState) UpdateSecret(arg0 *secrets.URI, arg1 state.UpdateSecretParams) (*secrets.SecretMetadata, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateSecret", arg0, arg1) + ret0, _ := ret[0].(*secrets.SecretMetadata) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateSecret indicates an expected call of UpdateSecret. +func (mr *MockSecretsStateMockRecorder) UpdateSecret(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSecret", reflect.TypeOf((*MockSecretsState)(nil).UpdateSecret), arg0, arg1) +} + // MockSecretsConsumer is a mock of SecretsConsumer interface. type MockSecretsConsumer struct { ctrl *gomock.Controller @@ -161,3 +212,18 @@ func (mr *MockSecretsConsumerMockRecorder) RevokeSecretAccess(arg0, arg1 interfa mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeSecretAccess", reflect.TypeOf((*MockSecretsConsumer)(nil).RevokeSecretAccess), arg0, arg1) } + +// SecretAccess mocks base method. +func (m *MockSecretsConsumer) SecretAccess(arg0 *secrets.URI, arg1 names.Tag) (secrets.SecretRole, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SecretAccess", arg0, arg1) + ret0, _ := ret[0].(secrets.SecretRole) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SecretAccess indicates an expected call of SecretAccess. +func (mr *MockSecretsConsumerMockRecorder) SecretAccess(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SecretAccess", reflect.TypeOf((*MockSecretsConsumer)(nil).SecretAccess), arg0, arg1) +} diff --git a/apiserver/facades/client/secrets/package_test.go b/apiserver/facades/client/secrets/package_test.go index 205896cddfe..658dc694bd9 100644 --- a/apiserver/facades/client/secrets/package_test.go +++ b/apiserver/facades/client/secrets/package_test.go @@ -6,6 +6,7 @@ package secrets import ( "testing" + "github.com/juju/names/v4" gc "gopkg.in/check.v1" apiservererrors "github.com/juju/juju/apiserver/errors" @@ -15,7 +16,7 @@ import ( ) //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/secretsstate.go github.com/juju/juju/apiserver/facades/client/secrets SecretsState,SecretsConsumer -//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/secretsbackend.go github.com/juju/juju/secrets/provider SecretsBackend +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/secretsbackend.go github.com/juju/juju/secrets/provider SecretsBackend,SecretBackendProvider func TestPackage(t *testing.T) { gc.TestingT(t) } @@ -25,13 +26,14 @@ func NewTestAPI( secretsConsumer SecretsConsumer, backendConfigGetter func() (*provider.ModelBackendConfigInfo, error), backendGetter func(*provider.ModelBackendConfig) (provider.SecretsBackend, error), - authorizer facade.Authorizer, + authorizer facade.Authorizer, authTag names.Tag, ) (*SecretsAPI, error) { if !authorizer.AuthClient() { return nil, apiservererrors.ErrPerm } return &SecretsAPI{ + authTag: authTag, authorizer: authorizer, controllerUUID: coretesting.ControllerTag.Id(), modelUUID: coretesting.ModelTag.Id(), diff --git a/apiserver/facades/client/secrets/register.go b/apiserver/facades/client/secrets/register.go index d1624d1930b..8c073a665fe 100644 --- a/apiserver/facades/client/secrets/register.go +++ b/apiserver/facades/client/secrets/register.go @@ -55,6 +55,7 @@ func newSecretsAPI(context facade.Context) (*SecretsAPI, error) { } return &SecretsAPI{ authorizer: context.Auth(), + authTag: context.Auth().GetAuthTag(), controllerUUID: context.State().ControllerUUID(), modelUUID: context.State().ModelUUID(), modelName: model.Name(), diff --git a/apiserver/facades/client/secrets/secrets.go b/apiserver/facades/client/secrets/secrets.go index 7ffd1a98693..2e4e0b3d421 100644 --- a/apiserver/facades/client/secrets/secrets.go +++ b/apiserver/facades/client/secrets/secrets.go @@ -11,8 +11,10 @@ import ( "github.com/juju/names/v4" "github.com/juju/juju/apiserver/common" + commonsecrets "github.com/juju/juju/apiserver/common/secrets" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" + "github.com/juju/juju/core/leadership" "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" @@ -28,6 +30,7 @@ var logger = loggo.GetLogger("juju.apiserver.client.secrets") // SecretsAPI is the backend for the Secrets facade. type SecretsAPI struct { authorizer facade.Authorizer + authTag names.Tag controllerUUID string modelUUID string modelName string @@ -221,16 +224,18 @@ func (s *SecretsAPI) secretContentFromBackend(uri *coresecrets.URI, rev int) (co } } -func (s *SecretsAPI) getActiveBackend() (provider.SecretsBackend, error) { +func (s *SecretsAPI) getBackend(id *string) (provider.SecretsBackend, error) { if s.activeBackendID == "" { - err := s.getBackendInfo() - if err != nil { + if err := s.getBackendInfo(); err != nil { return nil, errors.Trace(err) } } - backend, ok := s.backends[s.activeBackendID] + if id == nil { + id = &s.activeBackendID + } + backend, ok := s.backends[*id] if !ok { - return nil, errors.NotFoundf("external secret backend %q, have %q", s.activeBackendID, s.backends) + return nil, errors.NotFoundf("external secret backend %q", s.activeBackendID) } return backend, nil } @@ -246,7 +251,7 @@ func (s *SecretsAPI) CreateSecrets(args params.CreateSecretArgs) (params.StringR if err := s.checkCanAdmin(); err != nil { return result, errors.Trace(err) } - backend, err := s.getActiveBackend() + backend, err := s.getBackend(nil) if err != nil { return result, errors.Trace(err) } @@ -261,19 +266,25 @@ func (s *SecretsAPI) CreateSecrets(args params.CreateSecretArgs) (params.StringR return result, nil } -type successfulToken struct{} +type token struct { + disallow bool +} // Check implements lease.Token. -func (t successfulToken) Check() error { +func (t token) Check() error { + if t.disallow { + return apiservererrors.ErrPerm + } return nil } -func (s *SecretsAPI) createSecret(backend provider.SecretsBackend, arg params.CreateSecretArg) (_ string, err error) { +func (s *SecretsAPI) createSecret(backend provider.SecretsBackend, arg params.CreateSecretArg) (_ string, errOut error) { if arg.OwnerTag != "" && arg.OwnerTag != s.modelUUID { return "", errors.NotValidf("owner tag %q", arg.OwnerTag) } secretOwner := names.NewModelTag(s.modelUUID) var uri *coresecrets.URI + var err error if arg.URI != nil { uri, err = coresecrets.ParseURI(*arg.URI) if err != nil { @@ -292,13 +303,13 @@ func (s *SecretsAPI) createSecret(backend provider.SecretsBackend, arg params.Cr } if err == nil { defer func() { - if err != nil { + if errOut != nil { // If we failed to create the secret, we should delete the // secret value from the backend. - if err := backend.DeleteContent(context.TODO(), revId); err != nil && - !errors.Is(err, errors.NotSupported) && - !errors.Is(err, errors.NotFound) { - logger.Errorf("failed to delete secret %q: %v", revId, err) + if err2 := backend.DeleteContent(context.TODO(), revId); err2 != nil && + !errors.Is(err2, errors.NotSupported) && + !errors.Is(err2, errors.NotFound) { + logger.Errorf("failed to delete secret %q: %v", revId, err2) } } }() @@ -329,7 +340,7 @@ func fromUpsertParams(p params.UpsertSecretArg) state.UpdateSecretParams { } } return state.UpdateSecretParams{ - LeaderToken: successfulToken{}, + LeaderToken: token{}, Description: p.Description, Label: p.Label, Params: p.Params, @@ -338,7 +349,93 @@ func fromUpsertParams(p params.UpsertSecretArg) state.UpdateSecretParams { } } -// CreateSecrets isn't on the v1 API. +// UpdateSecrets isn't on the v1 API. +func (s *SecretsAPIV1) UpdateSecrets(_ struct{}) {} + +// UpdateSecrets creates new secrets. +func (s *SecretsAPI) UpdateSecrets(args params.UpdateUserSecretArgs) (params.ErrorResults, error) { + result := params.ErrorResults{ + Results: make([]params.ErrorResult, len(args.Args)), + } + if err := s.checkCanAdmin(); err != nil { + return result, errors.Trace(err) + } + backend, err := s.getBackend(nil) + if err != nil { + return result, errors.Trace(err) + } + for i, arg := range args.Args { + err := s.updateSecret(backend, arg) + if errors.Is(err, state.LabelExists) { + err = errors.AlreadyExistsf("secret with label %q", *arg.Label) + } + result.Results[i].Error = apiservererrors.ServerError(err) + } + return result, nil +} + +func (s *SecretsAPI) updateSecret(backend provider.SecretsBackend, arg params.UpdateUserSecretArg) (errOut error) { + uri, err := coresecrets.ParseURI(arg.URI) + if err != nil { + return errors.Trace(err) + } + + if arg.AutoPrune == nil && arg.Description == nil && arg.Label == nil && len(arg.Content.Data) == 0 { + return errors.New("at least one attribute to update must be specified") + } + md, err := s.secretsState.GetSecret(uri) + if err != nil { + // Check if the uri exists or not. + return errors.Trace(err) + } + if len(arg.Content.Data) > 0 { + revId, err := backend.SaveContent(context.TODO(), uri, md.LatestRevision+1, coresecrets.NewSecretValue(arg.Content.Data)) + if err != nil && !errors.Is(err, errors.NotSupported) { + return errors.Trace(err) + } + if err == nil { + defer func() { + if errOut != nil { + // If we failed to update the secret, we should delete the + // secret value from the backend for the new revision. + if err2 := backend.DeleteContent(context.TODO(), revId); err2 != nil && + !errors.Is(err2, errors.NotSupported) && + !errors.Is(err2, errors.NotFound) { + logger.Errorf("failed to delete secret %q: %v", revId, err2) + } + } + }() + arg.Content.Data = nil + arg.Content.ValueRef = ¶ms.SecretValueRef{ + BackendID: s.activeBackendID, + RevisionID: revId, + } + } + } + _, err = s.secretsState.UpdateSecret(uri, fromUpsertParams(arg.UpsertSecretArg)) + return errors.Trace(err) +} + +type leadershipChecker struct{} + +// Check implements lease.Token. +func (t leadershipChecker) LeadershipCheck(applicationId, unitId string) leadership.Token { + return &token{disallow: true} +} + +// RemoveSecrets isn't on the v1 API. +func (s *SecretsAPIV1) RemoveSecrets(_ struct{}) {} + +// RemoveSecrets remove user secret. +func (s *SecretsAPI) RemoveSecrets(args params.DeleteSecretArgs) (params.ErrorResults, error) { + return commonsecrets.RemoveSecrets( + s.secretsState, s.secretsState, s.secretsConsumer, s.backendConfigGetter, + names.NewModelTag(s.modelUUID), s.authorizer, s.authTag, &leadershipChecker{}, + args, true, + ) +} + +// GrantSecret isn't on the v1 API. func (s *SecretsAPIV1) GrantSecret(_ struct{}) {} // GrantSecret grants access to a user secret. @@ -373,7 +470,7 @@ func (s *SecretsAPI) secretsGrantRevoke(arg params.GrantRevokeUserSecretArg, op one := func(appName string) error { subjectTag := names.NewApplicationTag(appName) if err := op(uri, state.SecretAccessParams{ - LeaderToken: successfulToken{}, + LeaderToken: token{}, Scope: scopeTag, Subject: subjectTag, Role: coresecrets.RoleView, diff --git a/apiserver/facades/client/secrets/secrets_test.go b/apiserver/facades/client/secrets/secrets_test.go index 5f9a01166db..ef6b3599715 100644 --- a/apiserver/facades/client/secrets/secrets_test.go +++ b/apiserver/facades/client/secrets/secrets_test.go @@ -6,6 +6,7 @@ package secrets_test import ( "time" + "github.com/juju/collections/set" "github.com/juju/errors" "github.com/juju/names/v4" "github.com/juju/testing" @@ -14,6 +15,7 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/apiserver/authentication" + commonsecrets "github.com/juju/juju/apiserver/common/secrets" apiservererrors "github.com/juju/juju/apiserver/errors" facademocks "github.com/juju/juju/apiserver/facade/mocks" apisecrets "github.com/juju/juju/apiserver/facades/client/secrets" @@ -30,6 +32,9 @@ type SecretsSuite struct { testing.IsolationSuite authorizer *facademocks.MockAuthorizer + authTag names.Tag + provider *mocks.MockSecretBackendProvider + backend *mocks.MockSecretsBackend secretsState *mocks.MockSecretsState secretConsumer *mocks.MockSecretsConsumer secretsBackend *mocks.MockSecretsBackend @@ -39,6 +44,8 @@ var _ = gc.Suite(&SecretsSuite{}) func (s *SecretsSuite) SetUpTest(c *gc.C) { s.IsolationSuite.SetUpTest(c) + + s.authTag = names.NewUserTag("foo") } func (s *SecretsSuite) setup(c *gc.C) *gomock.Controller { @@ -48,7 +55,9 @@ func (s *SecretsSuite) setup(c *gc.C) *gomock.Controller { s.secretsState = mocks.NewMockSecretsState(ctrl) s.secretConsumer = mocks.NewMockSecretsConsumer(ctrl) s.secretsBackend = mocks.NewMockSecretsBackend(ctrl) - + s.provider = mocks.NewMockSecretBackendProvider(ctrl) + s.backend = mocks.NewMockSecretsBackend(ctrl) + s.PatchValue(&commonsecrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return s.provider, nil }) return ctrl } @@ -111,7 +120,7 @@ func (s *SecretsSuite) assertListSecrets(c *gc.C, reveal, withBackend bool) { func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) return s.secretsBackend, nil - }, s.authorizer) + }, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) now := time.Now() @@ -210,7 +219,7 @@ func (s *SecretsSuite) TestListSecretsPermissionDenied(c *gc.C) { s.authorizer.EXPECT().HasPermission(permission.ReadAccess, coretesting.ModelTag).Return( errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) - facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer) + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) _, err = facade.ListSecrets(params.ListSecretsArgs{}) @@ -226,7 +235,7 @@ func (s *SecretsSuite) TestListSecretsPermissionDeniedShow(c *gc.C) { s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return( errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) - facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer) + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) _, err = facade.ListSecrets(params.ListSecretsArgs{ShowSecrets: true}) @@ -242,7 +251,7 @@ func (s *SecretsSuite) TestCreateSecretsPermissionDenied(c *gc.C) { s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return( errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) - facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer) + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) _, err = facade.CreateSecrets(params.CreateSecretArgs{}) @@ -289,7 +298,7 @@ func (s *SecretsSuite) TestCreateSecretsEmptyData(c *gc.C) { func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) return s.secretsBackend, nil - }, s.authorizer) + }, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) result, err := facade.CreateSecrets(params.CreateSecretArgs{ @@ -374,7 +383,7 @@ func (s *SecretsSuite) assertCreateSecrets(c *gc.C, isInternal bool, finalStepFa func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) return s.secretsBackend, nil - }, s.authorizer) + }, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) result, err := facade.CreateSecrets(params.CreateSecretArgs{ @@ -412,6 +421,289 @@ func (s *SecretsSuite) TestCreateSecretsInternalBackend(c *gc.C) { s.assertCreateSecrets(c, true, false) } +func (s *SecretsSuite) assertUpdateSecrets(c *gc.C, isInternal bool, finalStepFailed bool) { + defer s.setup(c).Finish() + + s.expectAuthClient() + s.authorizer.EXPECT().HasPermission(permission.SuperuserAccess, coretesting.ControllerTag).Return( + errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) + + uri := coresecrets.NewURI() + s.secretsState.EXPECT().GetSecret(uri).Return(&coresecrets.SecretMetadata{ + URI: uri, + LatestRevision: 2, + }, nil) + if isInternal { + s.secretsBackend.EXPECT().SaveContent(gomock.Any(), uri, 3, coresecrets.NewSecretValue(map[string]string{"foo": "bar"})). + Return("", errors.NotSupportedf("not supported")) + } else { + s.secretsBackend.EXPECT().SaveContent(gomock.Any(), uri, 3, coresecrets.NewSecretValue(map[string]string{"foo": "bar"})). + Return("rev-id", nil) + } + s.secretsState.EXPECT().UpdateSecret(gomock.Any(), gomock.Any()).DoAndReturn(func(arg1 *coresecrets.URI, params state.UpdateSecretParams) (*coresecrets.SecretMetadata, error) { + c.Assert(arg1, gc.DeepEquals, uri) + c.Assert(params.Description, gc.DeepEquals, ptr("this is a user secret.")) + c.Assert(params.Label, gc.DeepEquals, ptr("label")) + if isInternal { + c.Assert(params.ValueRef, gc.IsNil) + c.Assert(params.Data, gc.DeepEquals, coresecrets.SecretData(map[string]string{"foo": "bar"})) + } else { + c.Assert(params.ValueRef, gc.DeepEquals, &coresecrets.ValueRef{ + BackendID: "backend-id", + RevisionID: "rev-id", + }) + c.Assert(params.Data, gc.IsNil) + } + if finalStepFailed { + return nil, errors.New("some error") + } + return &coresecrets.SecretMetadata{URI: uri}, nil + }) + if finalStepFailed && !isInternal { + s.secretsBackend.EXPECT().DeleteContent(gomock.Any(), "rev-id").Return(nil) + } + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + + result, err := facade.UpdateSecrets(params.UpdateUserSecretArgs{ + Args: []params.UpdateUserSecretArg{ + { + AutoPrune: ptr(true), + URI: uri.String(), + UpsertSecretArg: params.UpsertSecretArg{ + Description: ptr("this is a user secret."), + Label: ptr("label"), + Content: params.SecretContentParams{ + Data: map[string]string{"foo": "bar"}, + }, + }, + }, + }, + }) + c.Assert(err, jc.ErrorIsNil) + if finalStepFailed { + c.Assert(result.Results[0].Error.Message, gc.DeepEquals, "some error") + } else { + c.Assert(result.Results[0].Error, gc.IsNil) + } +} + +func (s *SecretsSuite) TestUpdateSecretsExternalBackend(c *gc.C) { + s.assertUpdateSecrets(c, false, false) +} + +func (s *SecretsSuite) TestUpdateSecretsExternalBackendFailedAndCleanup(c *gc.C) { + s.assertUpdateSecrets(c, false, true) +} + +func (s *SecretsSuite) TestUpdateSecretsInternalBackend(c *gc.C) { + s.assertUpdateSecrets(c, true, false) +} + +func (s *SecretsSuite) TestRemoveSecrets(c *gc.C) { + defer s.setup(c).Finish() + s.expectAuthClient() + + uri := coresecrets.NewURI() + expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ + BackendID: "backend-id", + RevisionID: "rev-666", + }}, nil) + + cfg := &provider.ModelBackendConfig{ + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + } + s.provider.EXPECT().NewBackend(cfg).Return(s.backend, nil) + s.backend.EXPECT().DeleteContent(gomock.Any(), "rev-666").Return(nil) + s.provider.EXPECT().CleanupSecrets( + cfg, names.NewUserTag("foo"), + provider.SecretRevisions{uri.ID: set.NewStrings("rev-666")}, + ).Return(nil) + + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + results, err := facade.RemoveSecrets(params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{ + Results: []params.ErrorResult{{}}, + }) +} + +func (s *SecretsSuite) TestRemoveSecretRevision(c *gc.C) { + defer s.setup(c).Finish() + s.expectAuthClient() + + uri := coresecrets.NewURI() + expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return(nil, nil) + + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + results, err := facade.RemoveSecrets(params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{ + Results: []params.ErrorResult{{}}, + }) +} + +func (s *SecretsSuite) TestRemoveSecretNotFound(c *gc.C) { + defer s.setup(c).Finish() + s.expectAuthClient() + + uri := coresecrets.NewURI() + expectURI := *uri + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, errors.NotFoundf("not found")) + + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + results, err := facade.RemoveSecrets(params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results.Results[0].Error, jc.Satisfies, params.IsCodeNotFound) +} + func (s *SecretsSuite) TestGrantSecret(c *gc.C) { defer s.setup(c).Finish() @@ -467,7 +759,7 @@ func (s *SecretsSuite) TestGrantSecret(c *gc.C) { func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) return s.secretsBackend, nil - }, s.authorizer) + }, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) result, err := facade.GrantSecret(params.GrantRevokeUserSecretArg{ @@ -488,7 +780,7 @@ func (s *SecretsSuite) TestGrantSecretPermissionDenied(c *gc.C) { errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission), ) - facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer) + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) _, err = facade.GrantSecret(params.GrantRevokeUserSecretArg{}) @@ -550,7 +842,7 @@ func (s *SecretsSuite) TestRevokeSecret(c *gc.C) { func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) return s.secretsBackend, nil - }, s.authorizer) + }, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) result, err := facade.RevokeSecret(params.GrantRevokeUserSecretArg{ @@ -571,7 +863,7 @@ func (s *SecretsSuite) TestRevokeSecretPermissionDenied(c *gc.C) { errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission), ) - facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer) + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, nil, nil, s.authorizer, s.authTag) c.Assert(err, jc.ErrorIsNil) _, err = facade.RevokeSecret(params.GrantRevokeUserSecretArg{}) diff --git a/apiserver/facades/client/secrets/state.go b/apiserver/facades/client/secrets/state.go index 023f90b2f20..09ba1d57d7b 100644 --- a/apiserver/facades/client/secrets/state.go +++ b/apiserver/facades/client/secrets/state.go @@ -4,6 +4,8 @@ package secrets import ( + "github.com/juju/names/v4" + "github.com/juju/juju/core/secrets" "github.com/juju/juju/state" ) @@ -11,8 +13,11 @@ import ( // SecretsState instances provide secret apis. type SecretsState interface { CreateSecret(*secrets.URI, state.CreateSecretParams) (*secrets.SecretMetadata, error) + UpdateSecret(*secrets.URI, state.UpdateSecretParams) (*secrets.SecretMetadata, error) + GetSecret(uri *secrets.URI) (*secrets.SecretMetadata, error) GetSecretValue(*secrets.URI, int) (secrets.SecretValue, *secrets.ValueRef, error) GetSecretRevision(uri *secrets.URI, revision int) (*secrets.SecretRevisionMetadata, error) + DeleteSecret(*secrets.URI, ...int) ([]secrets.ValueRef, error) ListSecrets(state.SecretsFilter) ([]*secrets.SecretMetadata, error) ListSecretRevisions(uri *secrets.URI) ([]*secrets.SecretRevisionMetadata, error) } @@ -21,4 +26,5 @@ type SecretsState interface { type SecretsConsumer interface { GrantSecretAccess(*secrets.URI, state.SecretAccessParams) error RevokeSecretAccess(*secrets.URI, state.SecretAccessParams) error + SecretAccess(uri *secrets.URI, subject names.Tag) (secrets.SecretRole, error) } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 813f4fedee0..d6349aa4aff 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -39889,6 +39889,18 @@ "Schema": { "type": "object", "properties": { + "CreateSecretURIs": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/CreateSecretURIsArg" + }, + "Result": { + "$ref": "#/definitions/StringResults" + } + }, + "description": "CreateSecretURIs creates new secret URIs." + }, "CreateSecrets": { "type": "object", "properties": { @@ -39901,6 +39913,63 @@ }, "description": "CreateSecrets creates new secrets." }, + "GetConsumerSecretsRevisionInfo": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GetSecretConsumerInfoArgs" + }, + "Result": { + "$ref": "#/definitions/SecretConsumerInfoResults" + } + }, + "description": "GetConsumerSecretsRevisionInfo returns the latest secret revisions for the specified secrets.\nThis facade method is used for remote watcher to get the latest secret revisions and labels for a secret changed hook." + }, + "GetSecretBackendConfigs": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/SecretBackendArgs" + }, + "Result": { + "$ref": "#/definitions/SecretBackendConfigResults" + } + }, + "description": "GetSecretBackendConfigs gets the config needed to create a client to secret backends." + }, + "GetSecretContentInfo": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GetSecretContentArgs" + }, + "Result": { + "$ref": "#/definitions/SecretContentResults" + } + }, + "description": "GetSecretContentInfo returns the secret values for the specified secrets." + }, + "GetSecretMetadata": { + "type": "object", + "properties": { + "Result": { + "$ref": "#/definitions/ListSecretResults" + } + }, + "description": "GetSecretMetadata returns metadata for the caller's secrets." + }, + "GetSecretRevisionContentInfo": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/SecretRevisionArg" + }, + "Result": { + "$ref": "#/definitions/SecretContentResults" + } + }, + "description": "GetSecretRevisionContentInfo returns the secret values for the specified secret revisions." + }, "GrantSecret": { "type": "object", "properties": { @@ -39925,6 +39994,18 @@ }, "description": "ListSecrets lists available secrets." }, + "RemoveSecrets": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/DeleteSecretArgs" + }, + "Result": { + "$ref": "#/definitions/ErrorResults" + } + }, + "description": "RemoveSecrets remove user secret." + }, "RevokeSecret": { "type": "object", "properties": { @@ -39936,6 +40017,102 @@ } }, "description": "RevokeSecret revokes access to a user secret." + }, + "SecretsGrant": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GrantRevokeSecretArgs" + }, + "Result": { + "$ref": "#/definitions/ErrorResults" + } + }, + "description": "SecretsGrant grants access to a secret for the specified subjects." + }, + "SecretsRevoke": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GrantRevokeSecretArgs" + }, + "Result": { + "$ref": "#/definitions/ErrorResults" + } + }, + "description": "SecretsRevoke revokes access to a secret for the specified subjects." + }, + "SecretsRotated": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/SecretRotatedArgs" + }, + "Result": { + "$ref": "#/definitions/ErrorResults" + } + }, + "description": "SecretsRotated records when secrets were last rotated." + }, + "UpdateSecrets": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/UpdateUserSecretArgs" + }, + "Result": { + "$ref": "#/definitions/ErrorResults" + } + }, + "description": "UpdateSecrets creates new secrets." + }, + "WatchConsumedSecretsChanges": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/Entities" + }, + "Result": { + "$ref": "#/definitions/StringsWatchResults" + } + }, + "description": "WatchConsumedSecretsChanges sets up a watcher to notify of changes to secret revisions for the specified consumers." + }, + "WatchObsolete": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/Entities" + }, + "Result": { + "$ref": "#/definitions/StringsWatchResult" + } + }, + "description": "WatchObsolete returns a watcher for notifying when:\n - a secret owned by the entity is deleted\n - a secret revision owed by the entity no longer\n has any consumers\n\nObsolete revisions results are \"uri/revno\" and deleted\nsecret results are \"uri\"." + }, + "WatchSecretRevisionsExpiryChanges": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/Entities" + }, + "Result": { + "$ref": "#/definitions/SecretTriggerWatchResult" + } + }, + "description": "WatchSecretRevisionsExpiryChanges sets up a watcher to notify of changes to secret revision expiry config." + }, + "WatchSecretsRotationChanges": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/Entities" + }, + "Result": { + "$ref": "#/definitions/SecretTriggerWatchResult" + } + }, + "description": "WatchSecretsRotationChanges sets up a watcher to notify of changes to secret rotation config." } }, "definitions": { @@ -39998,6 +40175,78 @@ "args" ] }, + "CreateSecretURIsArg": { + "type": "object", + "properties": { + "count": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "count" + ] + }, + "DeleteSecretArg": { + "type": "object", + "properties": { + "revisions": { + "type": "array", + "items": { + "type": "integer" + } + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri" + ] + }, + "DeleteSecretArgs": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "$ref": "#/definitions/DeleteSecretArg" + } + } + }, + "additionalProperties": false, + "required": [ + "args" + ] + }, + "Entities": { + "type": "object", + "properties": { + "entities": { + "type": "array", + "items": { + "$ref": "#/definitions/Entity" + } + } + }, + "additionalProperties": false, + "required": [ + "entities" + ] + }, + "Entity": { + "type": "object", + "properties": { + "tag": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "tag" + ] + }, "Error": { "type": "object", "properties": { @@ -40047,17 +40296,114 @@ "results" ] }, - "GrantRevokeUserSecretArg": { + "GetSecretConsumerInfoArgs": { "type": "object", "properties": { - "applications": { + "consumer-tag": { + "type": "string" + }, + "uris": { "type": "array", "items": { "type": "string" } - }, - "uri": { - "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "consumer-tag", + "uris" + ] + }, + "GetSecretContentArg": { + "type": "object", + "properties": { + "label": { + "type": "string" + }, + "peek": { + "type": "boolean" + }, + "refresh": { + "type": "boolean" + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri" + ] + }, + "GetSecretContentArgs": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "$ref": "#/definitions/GetSecretContentArg" + } + } + }, + "additionalProperties": false, + "required": [ + "args" + ] + }, + "GrantRevokeSecretArg": { + "type": "object", + "properties": { + "role": { + "type": "string" + }, + "scope-tag": { + "type": "string" + }, + "subject-tags": { + "type": "array", + "items": { + "type": "string" + } + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri", + "scope-tag", + "subject-tags", + "role" + ] + }, + "GrantRevokeSecretArgs": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "$ref": "#/definitions/GrantRevokeSecretArg" + } + } + }, + "additionalProperties": false, + "required": [ + "args" + ] + }, + "GrantRevokeUserSecretArg": { + "type": "object", + "properties": { + "applications": { + "type": "array", + "items": { + "type": "string" + } + }, + "uri": { + "type": "string" } }, "additionalProperties": false, @@ -40158,6 +40504,127 @@ "filter" ] }, + "SecretBackendArgs": { + "type": "object", + "properties": { + "backend-ids": { + "type": "array", + "items": { + "type": "string" + } + }, + "for-drain": { + "type": "boolean" + } + }, + "additionalProperties": false, + "required": [ + "for-drain", + "backend-ids" + ] + }, + "SecretBackendConfig": { + "type": "object", + "properties": { + "params": { + "type": "object", + "patternProperties": { + ".*": { + "type": "object", + "additionalProperties": true + } + } + }, + "type": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "type" + ] + }, + "SecretBackendConfigResult": { + "type": "object", + "properties": { + "config": { + "$ref": "#/definitions/SecretBackendConfig" + }, + "draining": { + "type": "boolean" + }, + "model-controller": { + "type": "string" + }, + "model-name": { + "type": "string" + }, + "model-uuid": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "model-controller", + "model-uuid", + "model-name", + "draining" + ] + }, + "SecretBackendConfigResults": { + "type": "object", + "properties": { + "active-id": { + "type": "string" + }, + "results": { + "type": "object", + "patternProperties": { + ".*": { + "$ref": "#/definitions/SecretBackendConfigResult" + } + } + } + }, + "additionalProperties": false, + "required": [ + "active-id" + ] + }, + "SecretConsumerInfoResult": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "label": { + "type": "string" + }, + "revision": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "revision", + "label" + ] + }, + "SecretConsumerInfoResults": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": { + "$ref": "#/definitions/SecretConsumerInfoResult" + } + } + }, + "additionalProperties": false, + "required": [ + "results" + ] + }, "SecretContentParams": { "type": "object", "properties": { @@ -40175,6 +40642,42 @@ }, "additionalProperties": false }, + "SecretContentResult": { + "type": "object", + "properties": { + "backend-config": { + "$ref": "#/definitions/SecretBackendConfigResult" + }, + "content": { + "$ref": "#/definitions/SecretContentParams" + }, + "error": { + "$ref": "#/definitions/Error" + }, + "latest-revision": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "content" + ] + }, + "SecretContentResults": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": { + "$ref": "#/definitions/SecretContentResult" + } + } + }, + "additionalProperties": false, + "required": [ + "results" + ] + }, "SecretRevision": { "type": "object", "properties": { @@ -40205,6 +40708,106 @@ "revision" ] }, + "SecretRevisionArg": { + "type": "object", + "properties": { + "pending-delete": { + "type": "boolean" + }, + "revisions": { + "type": "array", + "items": { + "type": "integer" + } + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri", + "revisions", + "pending-delete" + ] + }, + "SecretRotatedArg": { + "type": "object", + "properties": { + "original-revision": { + "type": "integer" + }, + "skip": { + "type": "boolean" + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri", + "original-revision", + "skip" + ] + }, + "SecretRotatedArgs": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "$ref": "#/definitions/SecretRotatedArg" + } + } + }, + "additionalProperties": false, + "required": [ + "args" + ] + }, + "SecretTriggerChange": { + "type": "object", + "properties": { + "next-trigger-time": { + "type": "string", + "format": "date-time" + }, + "revision": { + "type": "integer" + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "uri", + "next-trigger-time" + ] + }, + "SecretTriggerWatchResult": { + "type": "object", + "properties": { + "changes": { + "type": "array", + "items": { + "$ref": "#/definitions/SecretTriggerChange" + } + }, + "error": { + "$ref": "#/definitions/Error" + }, + "watcher-id": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "watcher-id", + "changes" + ] + }, "SecretValueRef": { "type": "object", "properties": { @@ -40283,6 +40886,101 @@ "results" ] }, + "StringsWatchResult": { + "type": "object", + "properties": { + "changes": { + "type": "array", + "items": { + "type": "string" + } + }, + "error": { + "$ref": "#/definitions/Error" + }, + "watcher-id": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "watcher-id" + ] + }, + "StringsWatchResults": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": { + "$ref": "#/definitions/StringsWatchResult" + } + } + }, + "additionalProperties": false, + "required": [ + "results" + ] + }, + "UpdateUserSecretArg": { + "type": "object", + "properties": { + "UpsertSecretArg": { + "$ref": "#/definitions/UpsertSecretArg" + }, + "auto-prune": { + "type": "boolean" + }, + "content": { + "$ref": "#/definitions/SecretContentParams" + }, + "description": { + "type": "string" + }, + "expire-time": { + "type": "string", + "format": "date-time" + }, + "label": { + "type": "string" + }, + "params": { + "type": "object", + "patternProperties": { + ".*": { + "type": "object", + "additionalProperties": true + } + } + }, + "rotate-policy": { + "type": "string" + }, + "uri": { + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "UpsertSecretArg", + "uri" + ] + }, + "UpdateUserSecretArgs": { + "type": "object", + "properties": { + "args": { + "type": "array", + "items": { + "$ref": "#/definitions/UpdateUserSecretArg" + } + } + }, + "additionalProperties": false, + "required": [ + "args" + ] + }, "UpsertSecretArg": { "type": "object", "properties": { From bbf8f26b35f2562ed747550651f3df9b41222dcc Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:28:39 +1000 Subject: [PATCH 03/10] Implement client facade methods: UpdateSecret and RemoveSecret; --- api/client/secrets/client.go | 63 ++++++++++++++++++++++++++++ api/client/secrets/client_test.go | 70 +++++++++++++++++++++++++++++++ rpc/params/secrets.go | 16 +++++++ 3 files changed, 149 insertions(+) diff --git a/api/client/secrets/client.go b/api/client/secrets/client.go index 5f20f1b124a..d73a0881d57 100644 --- a/api/client/secrets/client.go +++ b/api/client/secrets/client.go @@ -4,6 +4,7 @@ package secrets import ( + "fmt" "github.com/juju/errors" "github.com/juju/juju/api/base" @@ -126,6 +127,68 @@ func (c *Client) CreateSecret(label, description string, data map[string]string) return result.Result, nil } +func (c *Client) UpdateSecret( + uri *secrets.URI, autoPrune *bool, + label, description string, data map[string]string, +) error { + if c.BestAPIVersion() < 2 { + return errors.NotSupportedf("user secrets") + } + var results params.ErrorResults + arg := params.UpdateUserSecretArg{ + URI: uri.String(), + AutoPrune: autoPrune, + UpsertSecretArg: params.UpsertSecretArg{ + Content: params.SecretContentParams{Data: data}, + }, + } + if label != "" { + arg.UpsertSecretArg.Label = &label + } + if description != "" { + arg.UpsertSecretArg.Description = &description + } + fmt.Println(arg) + err := c.facade.FacadeCall("UpdateSecrets", params.UpdateUserSecretArgs{Args: []params.UpdateUserSecretArg{arg}}, &results) + if err != nil { + return errors.Trace(err) + } + if len(results.Results) != 1 { + return errors.Errorf("expected 1 result, got %d", len(results.Results)) + } + result := results.Results[0] + if result.Error != nil { + return params.TranslateWellKnownError(result.Error) + } + return nil +} + +func (c *Client) RemoveSecret(uri *secrets.URI, revision *int) error { + if c.BestAPIVersion() < 2 { + return errors.NotSupportedf("user secrets") + } + arg := params.DeleteSecretArg{ + URI: uri.String(), + } + if revision != nil { + arg.Revisions = append(arg.Revisions, *revision) + } + + var results params.ErrorResults + err := c.facade.FacadeCall("RemoveSecrets", params.DeleteSecretArgs{Args: []params.DeleteSecretArg{arg}}, &results) + if err != nil { + return errors.Trace(err) + } + if len(results.Results) != 1 { + return errors.Errorf("expected 1 result, got %d", len(results.Results)) + } + result := results.Results[0] + if result.Error != nil { + return params.TranslateWellKnownError(result.Error) + } + return nil +} + // GrantSecret grants access to a secret to the specified applications. func (c *Client) GrantSecret(uri *secrets.URI, apps []string) ([]error, error) { if c.BestAPIVersion() < 2 { diff --git a/api/client/secrets/client_test.go b/api/client/secrets/client_test.go index ae82311a9ad..02f567adaf7 100644 --- a/api/client/secrets/client_test.go +++ b/api/client/secrets/client_test.go @@ -175,6 +175,76 @@ func (s *SecretsSuite) TestCreateSecret(c *gc.C) { c.Assert(result, gc.DeepEquals, uri.String()) } +func (s *SecretsSuite) TestUpdateSecretError(c *gc.C) { + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 1} + client := apisecrets.NewClient(caller) + uri := secrets.NewURI() + err := client.UpdateSecret(uri, ptr(true), "label", "this is a secret.", map[string]string{"foo": "bar"}) + c.Assert(err, gc.ErrorMatches, "user secrets not supported") +} + +func (s *SecretsSuite) TestUpdateSecret(c *gc.C) { + uri := secrets.NewURI() + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + c.Assert(objType, gc.Equals, "Secrets") + c.Assert(request, gc.Equals, "UpdateSecrets") + c.Assert(arg, gc.DeepEquals, params.UpdateUserSecretArgs{ + Args: []params.UpdateUserSecretArg{ + { + URI: uri.String(), + AutoPrune: ptr(true), + UpsertSecretArg: params.UpsertSecretArg{ + Label: ptr("label"), + Description: ptr("this is a secret."), + Content: params.SecretContentParams{Data: map[string]string{"foo": "bar"}}, + }, + }, + }, + }) + *(result.(*params.ErrorResults)) = params.ErrorResults{Results: []params.ErrorResult{{}}} + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 2} + client := apisecrets.NewClient(caller) + err := client.UpdateSecret(uri, ptr(true), "label", "this is a secret.", map[string]string{"foo": "bar"}) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *SecretsSuite) TestRemoveSecretError(c *gc.C) { + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 1} + client := apisecrets.NewClient(caller) + uri := secrets.NewURI() + err := client.RemoveSecret(uri, ptr(1)) + c.Assert(err, gc.ErrorMatches, "user secrets not supported") +} + +func (s *SecretsSuite) TestRemoveSecret(c *gc.C) { + uri := secrets.NewURI() + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + c.Assert(objType, gc.Equals, "Secrets") + c.Assert(request, gc.Equals, "RemoveSecrets") + c.Assert(arg, gc.DeepEquals, params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{ + {URI: uri.String(), Revisions: []int{1}}, + }, + }) + *(result.(*params.ErrorResults)) = params.ErrorResults{ + Results: []params.ErrorResult{{}}, + } + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 2} + client := apisecrets.NewClient(caller) + err := client.RemoveSecret(uri, ptr(1)) + c.Assert(err, jc.ErrorIsNil) +} + func (s *SecretsSuite) TestGrantSecretError(c *gc.C) { apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { return nil diff --git a/rpc/params/secrets.go b/rpc/params/secrets.go index 3338b3d5dd0..c5775bfd1e9 100644 --- a/rpc/params/secrets.go +++ b/rpc/params/secrets.go @@ -111,6 +111,22 @@ type UpdateSecretArg struct { URI string `json:"uri"` } +// UpdateUserSecretArgs holds args for updating user secrets. +type UpdateUserSecretArgs struct { + Args []UpdateUserSecretArg `json:"args"` +} + +// UpdateUserSecretArg holds the args for updating a user secret. +type UpdateUserSecretArg struct { + UpsertSecretArg + + // URI identifies the secret to update. + URI string `json:"uri"` + + // AutoPrune indicates whether the staled secret revisions should be pruned automatically. + AutoPrune *bool `json:"auto-prune,omitempty"` +} + // DeleteSecretArgs holds args for deleting secrets. type DeleteSecretArgs struct { Args []DeleteSecretArg `json:"args"` From 1a597e3b519834482fe782759cb14ca1a24053d0 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:32:53 +1000 Subject: [PATCH 04/10] Implement update-secret and remove-secret commands; --- cmd/juju/commands/main.go | 2 + cmd/juju/secrets/mocks/secretsapi.go | 104 ++++++++++++++++++++++++- cmd/juju/secrets/package_test.go | 20 ++++- cmd/juju/secrets/remove.go | 93 ++++++++++++++++++++++ cmd/juju/secrets/remove_test.go | 57 ++++++++++++++ cmd/juju/secrets/update.go | 112 +++++++++++++++++++++++++++ cmd/juju/secrets/update_test.go | 84 ++++++++++++++++++++ 7 files changed, 470 insertions(+), 2 deletions(-) create mode 100644 cmd/juju/secrets/remove.go create mode 100644 cmd/juju/secrets/remove_test.go create mode 100644 cmd/juju/secrets/update.go create mode 100644 cmd/juju/secrets/update_test.go diff --git a/cmd/juju/commands/main.go b/cmd/juju/commands/main.go index 5e67a2320e0..ac60e0c9b43 100644 --- a/cmd/juju/commands/main.go +++ b/cmd/juju/commands/main.go @@ -578,6 +578,8 @@ func registerCommands(r commandRegistry) { r.Register(secrets.NewListSecretsCommand()) r.Register(secrets.NewShowSecretsCommand()) r.Register(secrets.NewAddSecretCommand()) + r.Register(secrets.NewUpdateSecretCommand()) + r.Register(secrets.NewRemoveSecretCommand()) r.Register(secrets.NewGrantSecretCommand()) r.Register(secrets.NewRevokeSecretCommand()) diff --git a/cmd/juju/secrets/mocks/secretsapi.go b/cmd/juju/secrets/mocks/secretsapi.go index 89119e1f25e..7d8d3cebe38 100644 --- a/cmd/juju/secrets/mocks/secretsapi.go +++ b/cmd/juju/secrets/mocks/secretsapi.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/cmd/juju/secrets (interfaces: ListSecretsAPI,AddSecretsAPI,GrantRevokeSecretsAPI) +// Source: github.com/juju/juju/cmd/juju/secrets (interfaces: ListSecretsAPI,AddSecretsAPI,GrantRevokeSecretsAPI,UpdateSecretsAPI,RemoveSecretsAPI) // Package mocks is a generated GoMock package. package mocks @@ -182,3 +182,105 @@ func (mr *MockGrantRevokeSecretsAPIMockRecorder) RevokeSecret(arg0, arg1 interfa mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeSecret", reflect.TypeOf((*MockGrantRevokeSecretsAPI)(nil).RevokeSecret), arg0, arg1) } + +// MockUpdateSecretsAPI is a mock of UpdateSecretsAPI interface. +type MockUpdateSecretsAPI struct { + ctrl *gomock.Controller + recorder *MockUpdateSecretsAPIMockRecorder +} + +// MockUpdateSecretsAPIMockRecorder is the mock recorder for MockUpdateSecretsAPI. +type MockUpdateSecretsAPIMockRecorder struct { + mock *MockUpdateSecretsAPI +} + +// NewMockUpdateSecretsAPI creates a new mock instance. +func NewMockUpdateSecretsAPI(ctrl *gomock.Controller) *MockUpdateSecretsAPI { + mock := &MockUpdateSecretsAPI{ctrl: ctrl} + mock.recorder = &MockUpdateSecretsAPIMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUpdateSecretsAPI) EXPECT() *MockUpdateSecretsAPIMockRecorder { + return m.recorder +} + +// Close mocks base method. +func (m *MockUpdateSecretsAPI) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockUpdateSecretsAPIMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockUpdateSecretsAPI)(nil).Close)) +} + +// UpdateSecret mocks base method. +func (m *MockUpdateSecretsAPI) UpdateSecret(arg0 *secrets0.URI, arg1 *bool, arg2, arg3 string, arg4 map[string]string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateSecret", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateSecret indicates an expected call of UpdateSecret. +func (mr *MockUpdateSecretsAPIMockRecorder) UpdateSecret(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSecret", reflect.TypeOf((*MockUpdateSecretsAPI)(nil).UpdateSecret), arg0, arg1, arg2, arg3, arg4) +} + +// MockRemoveSecretsAPI is a mock of RemoveSecretsAPI interface. +type MockRemoveSecretsAPI struct { + ctrl *gomock.Controller + recorder *MockRemoveSecretsAPIMockRecorder +} + +// MockRemoveSecretsAPIMockRecorder is the mock recorder for MockRemoveSecretsAPI. +type MockRemoveSecretsAPIMockRecorder struct { + mock *MockRemoveSecretsAPI +} + +// NewMockRemoveSecretsAPI creates a new mock instance. +func NewMockRemoveSecretsAPI(ctrl *gomock.Controller) *MockRemoveSecretsAPI { + mock := &MockRemoveSecretsAPI{ctrl: ctrl} + mock.recorder = &MockRemoveSecretsAPIMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRemoveSecretsAPI) EXPECT() *MockRemoveSecretsAPIMockRecorder { + return m.recorder +} + +// Close mocks base method. +func (m *MockRemoveSecretsAPI) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockRemoveSecretsAPIMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockRemoveSecretsAPI)(nil).Close)) +} + +// RemoveSecret mocks base method. +func (m *MockRemoveSecretsAPI) RemoveSecret(arg0 *secrets0.URI, arg1 *int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveSecret", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveSecret indicates an expected call of RemoveSecret. +func (mr *MockRemoveSecretsAPIMockRecorder) RemoveSecret(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveSecret", reflect.TypeOf((*MockRemoveSecretsAPI)(nil).RemoveSecret), arg0, arg1) +} diff --git a/cmd/juju/secrets/package_test.go b/cmd/juju/secrets/package_test.go index 00ccc062977..a9292e6ecb0 100644 --- a/cmd/juju/secrets/package_test.go +++ b/cmd/juju/secrets/package_test.go @@ -11,7 +11,7 @@ import ( "github.com/juju/juju/jujuclient" ) -//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/secretsapi.go github.com/juju/juju/cmd/juju/secrets ListSecretsAPI,AddSecretsAPI,GrantRevokeSecretsAPI +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/secretsapi.go github.com/juju/juju/cmd/juju/secrets ListSecretsAPI,AddSecretsAPI,GrantRevokeSecretsAPI,UpdateSecretsAPI,RemoveSecretsAPI func TestPackage(t *stdtesting.T) { gc.TestingT(t) @@ -26,6 +26,24 @@ func NewAddCommandForTest(store jujuclient.ClientStore, api AddSecretsAPI) *addS return c } +// NewUpdateCommandForTest returns a secrets command for testing. +func NewUpdateCommandForTest(store jujuclient.ClientStore, api UpdateSecretsAPI) *updateSecretCommand { + c := &updateSecretCommand{ + secretsAPIFunc: func() (UpdateSecretsAPI, error) { return api, nil }, + } + c.SetClientStore(store) + return c +} + +// NewRemoveCommandForTest returns a secrets command for testing. +func NewRemoveCommandForTest(store jujuclient.ClientStore, api RemoveSecretsAPI) *removeSecretCommand { + c := &removeSecretCommand{ + secretsAPIFunc: func() (RemoveSecretsAPI, error) { return api, nil }, + } + c.SetClientStore(store) + return c +} + // NewGrantCommandForTest returns a secrets command for testing. func NewGrantCommandForTest(store jujuclient.ClientStore, api GrantRevokeSecretsAPI) *grantSecretCommand { c := &grantSecretCommand{ diff --git a/cmd/juju/secrets/remove.go b/cmd/juju/secrets/remove.go new file mode 100644 index 00000000000..997125aa87a --- /dev/null +++ b/cmd/juju/secrets/remove.go @@ -0,0 +1,93 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package secrets + +import ( + "github.com/juju/cmd/v3" + "github.com/juju/errors" + "github.com/juju/gnuflag" + + apisecrets "github.com/juju/juju/api/client/secrets" + jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/core/secrets" +) + +type removeSecretCommand struct { + modelcmd.ModelCommandBase + + secretsAPIFunc func() (RemoveSecretsAPI, error) + + secretURI *secrets.URI + revision int +} + +// RemoveSecretsAPI is the secrets client API. +type RemoveSecretsAPI interface { + RemoveSecret(uri *secrets.URI, revision *int) error + Close() error +} + +// NewRemoveSecretCommand returns a command to remove a secret. +func NewRemoveSecretCommand() cmd.Command { + c := &removeSecretCommand{} + c.secretsAPIFunc = c.secretsAPI + return modelcmd.Wrap(c) +} + +func (c *removeSecretCommand) secretsAPI() (RemoveSecretsAPI, error) { + root, err := c.NewAPIRoot() + if err != nil { + return nil, errors.Trace(err) + } + return apisecrets.NewClient(root), nil +} + +// Info implements cmd.Command. +func (c *removeSecretCommand) Info() *cmd.Info { + doc := ` +Remove all the revisions of a secret with the specified URI or remove the provided revision only. + +Examples: + remove-secret secret:9m4e2mr0ui3e8a215n4g + remove-secret secret:9m4e2mr0ui3e8a215n4g --revision 4 +` + return jujucmd.Info(&cmd.Info{ + Name: "remove-secret", + Args: "", + Purpose: "Remove a existing secret.", + Doc: doc, + }) +} + +// SetFlags implements cmd.Command. +func (c *removeSecretCommand) SetFlags(f *gnuflag.FlagSet) { + f.IntVar(&c.revision, "revision", 0, "remove the specified revision") +} + +// Init implements cmd.Command. +func (c *removeSecretCommand) Init(args []string) error { + if len(args) < 1 { + return errors.New("missing secret URI") + } + var err error + if c.secretURI, err = secrets.ParseURI(args[0]); err != nil { + return errors.Trace(err) + } + return cmd.CheckEmpty(args[1:]) +} + +// Run implements cmd.Command. +func (c *removeSecretCommand) Run(ctx *cmd.Context) error { + var rev *int + if c.revision > 0 { + rev = &c.revision + } + secretsAPI, err := c.secretsAPIFunc() + if err != nil { + return errors.Trace(err) + } + defer secretsAPI.Close() + return secretsAPI.RemoveSecret(c.secretURI, rev) +} diff --git a/cmd/juju/secrets/remove_test.go b/cmd/juju/secrets/remove_test.go new file mode 100644 index 00000000000..12d726ae6a5 --- /dev/null +++ b/cmd/juju/secrets/remove_test.go @@ -0,0 +1,57 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package secrets_test + +import ( + "github.com/juju/cmd/v3/cmdtesting" + jujutesting "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + "go.uber.org/mock/gomock" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/cmd/juju/secrets" + "github.com/juju/juju/cmd/juju/secrets/mocks" + coresecrets "github.com/juju/juju/core/secrets" + "github.com/juju/juju/jujuclient" +) + +type removeSuite struct { + jujutesting.IsolationSuite + store *jujuclient.MemStore + secretsAPI *mocks.MockRemoveSecretsAPI +} + +var _ = gc.Suite(&removeSuite{}) + +func (s *removeSuite) SetUpTest(c *gc.C) { + s.IsolationSuite.SetUpTest(c) + store := jujuclient.NewMemStore() + store.Controllers["mycontroller"] = jujuclient.ControllerDetails{} + store.CurrentControllerName = "mycontroller" + s.store = store +} + +func (s *removeSuite) setup(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.secretsAPI = mocks.NewMockRemoveSecretsAPI(ctrl) + return ctrl +} + +func (s *removeSuite) TestRemoveMissArg(c *gc.C) { + defer s.setup(c).Finish() + + _, err := cmdtesting.RunCommand(c, secrets.NewRemoveCommandForTest(s.store, s.secretsAPI), "--revision", "4") + c.Assert(err, gc.ErrorMatches, `missing secret URI`) +} + +func (s *removeSuite) TestRemove(c *gc.C) { + defer s.setup(c).Finish() + + uri := coresecrets.NewURI() + s.secretsAPI.EXPECT().RemoveSecret(uri, ptr(4)).Return(nil) + s.secretsAPI.EXPECT().Close().Return(nil) + + _, err := cmdtesting.RunCommand(c, secrets.NewRemoveCommandForTest(s.store, s.secretsAPI), uri.String(), "--revision", "4") + c.Assert(err, jc.ErrorIsNil) +} diff --git a/cmd/juju/secrets/update.go b/cmd/juju/secrets/update.go new file mode 100644 index 00000000000..dc42bf0629c --- /dev/null +++ b/cmd/juju/secrets/update.go @@ -0,0 +1,112 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package secrets + +import ( + "github.com/juju/cmd/v3" + "github.com/juju/errors" + "github.com/juju/gnuflag" + + apisecrets "github.com/juju/juju/api/client/secrets" + jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/core/secrets" +) + +type updateSecretCommand struct { + modelcmd.ModelCommandBase + + SecretUpsertContentCommand + secretsAPIFunc func() (UpdateSecretsAPI, error) + + secretURI *secrets.URI + autoPrune bool +} + +// UpdateSecretsAPI is the secrets client API. +type UpdateSecretsAPI interface { + UpdateSecret( + uri *secrets.URI, autoPrune *bool, + label, description string, data map[string]string, + ) error + Close() error +} + +// NewUpdateSecretCommand returns a command to update a secret. +func NewUpdateSecretCommand() cmd.Command { + c := &updateSecretCommand{} + c.secretsAPIFunc = c.secretsAPI + return modelcmd.Wrap(c) +} + +func (c *updateSecretCommand) secretsAPI() (UpdateSecretsAPI, error) { + root, err := c.NewAPIRoot() + if err != nil { + return nil, errors.Trace(err) + } + return apisecrets.NewClient(root), nil +} + +// Info implements cmd.Command. +func (c *updateSecretCommand) Info() *cmd.Info { + doc := ` +Update a secret with a list of key values, or info. +If a value has the '#base64' suffix, it is already in base64 format and no +encoding will be performed, otherwise the value will be base64 encoded +prior to being stored. +The --auto-prune option is used to allow Juju to automatically remove revisions +which are no longer being tracked by any observers (see Rotation and Expiry). +This is configured per revision. This feature is opt-in because Juju +automatically removing secret content might result in data loss. + +Examples: + update-secret secret:9m4e2mr0ui3e8a215n4g token=34ae35facd4 + update-secret secret:9m4e2mr0ui3e8a215n4g token=34ae35facd4 --auto-prune + update-secret secret:9m4e2mr0ui3e8a215n4g key#base64 AA== + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ + --info "my database password" \ + data#base64 s3cret== + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ + --info "my database password" + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ + --info "my database password" \ + --file=/path/to/file +` + return jujucmd.Info(&cmd.Info{ + Name: "update-secret", + Args: " [key[#base64|#file]=value...]", + Purpose: "Update an exisitng secret.", + Doc: doc, + }) +} + +// Init implements cmd.Command. +func (c *updateSecretCommand) Init(args []string) error { + if len(args) < 1 { + return errors.New("missing secret URI") + } + var err error + if c.secretURI, err = secrets.ParseURI(args[0]); err != nil { + return errors.Trace(err) + } + return c.SecretUpsertContentCommand.Init(args[1:]) +} + +func (c *updateSecretCommand) SetFlags(f *gnuflag.FlagSet) { + c.SecretUpsertContentCommand.SetFlags(f) + f.BoolVar( + &c.autoPrune, "auto-prune", false, + "used to allow Juju to automatically remove revisions which are no longer being tracked by any observers", + ) +} + +// Run implements cmd.Command. +func (c *updateSecretCommand) Run(ctx *cmd.Context) error { + secretsAPI, err := c.secretsAPIFunc() + if err != nil { + return errors.Trace(err) + } + defer secretsAPI.Close() + return secretsAPI.UpdateSecret(c.secretURI, &c.autoPrune, c.Label, c.Description, c.Data) +} diff --git a/cmd/juju/secrets/update_test.go b/cmd/juju/secrets/update_test.go new file mode 100644 index 00000000000..9aec9c4b549 --- /dev/null +++ b/cmd/juju/secrets/update_test.go @@ -0,0 +1,84 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package secrets_test + +import ( + "os" + "path/filepath" + + "github.com/juju/cmd/v3/cmdtesting" + jujutesting "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + "go.uber.org/mock/gomock" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/cmd/juju/secrets" + "github.com/juju/juju/cmd/juju/secrets/mocks" + coresecrets "github.com/juju/juju/core/secrets" + "github.com/juju/juju/jujuclient" +) + +type updateSuite struct { + jujutesting.IsolationSuite + store *jujuclient.MemStore + secretsAPI *mocks.MockUpdateSecretsAPI +} + +var _ = gc.Suite(&updateSuite{}) + +func (s *updateSuite) SetUpTest(c *gc.C) { + s.IsolationSuite.SetUpTest(c) + store := jujuclient.NewMemStore() + store.Controllers["mycontroller"] = jujuclient.ControllerDetails{} + store.CurrentControllerName = "mycontroller" + s.store = store +} + +func (s *updateSuite) setup(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.secretsAPI = mocks.NewMockUpdateSecretsAPI(ctrl) + return ctrl +} + +func (s *updateSuite) TestUpdateMissArg(c *gc.C) { + defer s.setup(c).Finish() + + _, err := cmdtesting.RunCommand(c, secrets.NewUpdateCommandForTest(s.store, s.secretsAPI), "--label", "label", "--info", "this is a secret.") + c.Assert(err, gc.ErrorMatches, `missing secret URI`) +} + +func (s *updateSuite) TestUpdateFromArg(c *gc.C) { + defer s.setup(c).Finish() + + uri := coresecrets.NewURI() + s.secretsAPI.EXPECT().UpdateSecret(uri, ptr(true), "label", "this is a secret.", map[string]string{"foo": "YmFy"}).Return(nil) + s.secretsAPI.EXPECT().Close().Return(nil) + + _, err := cmdtesting.RunCommand(c, secrets.NewUpdateCommandForTest( + s.store, s.secretsAPI), uri.String(), "foo=bar", + "--auto-prune", "--label", "label", "--info", "this is a secret.", + ) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *updateSuite) TestUpdateFromFile(c *gc.C) { + defer s.setup(c).Finish() + + uri := coresecrets.NewURI() + s.secretsAPI.EXPECT().UpdateSecret(uri, ptr(true), "label", "this is a secret.", map[string]string{"foo": "YmFy"}).Return(nil) + s.secretsAPI.EXPECT().Close().Return(nil) + + dir := c.MkDir() + path := filepath.Join(dir, "data.txt") + data := ` +foo: bar + ` + err := os.WriteFile(path, []byte(data), 0644) + c.Assert(err, jc.ErrorIsNil) + _, err = cmdtesting.RunCommand(c, secrets.NewUpdateCommandForTest( + s.store, s.secretsAPI), uri.String(), "--file", path, + "--auto-prune", "--label", "label", "--info", "this is a secret.", + ) + c.Assert(err, jc.ErrorIsNil) +} From d5f4cd172a67f5cee806e41c56f5f8491d4259b5 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:33:35 +1000 Subject: [PATCH 05/10] Generate docs for update-secret and remove-secret commands; --- .github/discourse-topic-ids.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/discourse-topic-ids.yaml b/.github/discourse-topic-ids.yaml index 1b2f213e17f..b1708e47c6c 100644 --- a/.github/discourse-topic-ids.yaml +++ b/.github/discourse-topic-ids.yaml @@ -122,6 +122,7 @@ remove-machine: 10163 remove-offer: 10235 remove-relation: 10110 remove-saas: 10087 +remove-secret: 11414 remove-secret-backend: 10194 remove-space: 10084 remove-ssh-key: 10119 @@ -187,6 +188,7 @@ update-credential: 10065 update-credentials: 10231 update-k8s: 10155 update-public-clouds: 10115 +update-secret: 11413 update-secret-backend: 10176 update-storage-pool: 10217 upgrade-controller: 10058 From 36d7ea91fd9f77fc67195a05964ffeb38286e2ff Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Fri, 4 Aug 2023 16:00:33 +0100 Subject: [PATCH 06/10] Ensure risk is dropped when resolving charms Deduce platform should ensure the platform it returns doesn't include a risk. This is because charmhub doesn't really recognise the risk construct --- .../client/application/deployrepository.go | 14 ++++++++--- .../application/deployrepository_test.go | 24 ++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 24f066c304e..4ed55b8e97c 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -421,8 +421,14 @@ func (v *deployFromRepositoryValidator) deducePlatform(arg params.DeployFromRepo } } if argBase != nil { - platform.OS = argBase.Name - platform.Channel = argBase.Channel + base, err := corebase.ParseBase(argBase.Name, argBase.Channel) + if err != nil { + return corecharm.Platform{}, usedModelDefaultBase, err + } + platform.OS = base.OS + // platform channels don't model the concept of a risk + // so ensure that only the track is included + platform.Channel = base.Channel.Track } // Initial validation of platform from known data. @@ -455,7 +461,9 @@ func (v *deployFromRepositoryValidator) deducePlatform(arg params.DeployFromRepo return corecharm.Platform{}, usedModelDefaultBase, err } platform.OS = defaultBase.OS - platform.Channel = defaultBase.Channel.String() + // platform channels don't model the concept of a risk + // so ensure that only the track is included + platform.Channel = defaultBase.Channel.Track usedModelDefaultBase = true } } diff --git a/apiserver/facades/client/application/deployrepository_test.go b/apiserver/facades/client/application/deployrepository_test.go index 6c7df0d616f..5e492b11177 100644 --- a/apiserver/facades/client/application/deployrepository_test.go +++ b/apiserver/facades/client/application/deployrepository_test.go @@ -462,6 +462,28 @@ func (s *validatorSuite) TestDeducePlatformSimple(c *gc.C) { c.Assert(plat, gc.DeepEquals, corecharm.Platform{Architecture: "amd64"}) } +func (s *validatorSuite) TestDeducePlatformRiskInChannel(c *gc.C) { + defer s.setupMocks(c).Finish() + //model constraint default + s.state.EXPECT().ModelConstraints().Return(constraints.Value{Arch: strptr("amd64")}, nil) + + arg := params.DeployFromRepositoryArg{ + CharmName: "testme", + Base: ¶ms.Base{ + Name: "ubuntu", + Channel: "22.10/stable", + }, + } + plat, usedModelDefaultBase, err := s.getValidator().deducePlatform(arg) + c.Assert(err, gc.IsNil) + c.Assert(usedModelDefaultBase, jc.IsFalse) + c.Assert(plat, gc.DeepEquals, corecharm.Platform{ + Architecture: "amd64", + OS: "ubuntu", + Channel: "22.10", + }) +} + func (s *validatorSuite) TestDeducePlatformArgArchBase(c *gc.C) { defer s.setupMocks(c).Finish() @@ -504,7 +526,7 @@ func (s *validatorSuite) TestDeducePlatformModelDefaultBase(c *gc.C) { c.Assert(plat, gc.DeepEquals, corecharm.Platform{ Architecture: "amd64", OS: "ubuntu", - Channel: "22.04/stable", + Channel: "22.04", }) } From 371fc5bc6fc39ae66197e362bc65d7c79385a54d Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Fri, 11 Aug 2023 19:44:53 +1000 Subject: [PATCH 07/10] Re-generate schema.json; --- api/client/secrets/client.go | 1 + apiserver/facades/schema.json | 674 +++------------------------------ cmd/juju/commands/main_test.go | 2 + cmd/juju/secrets/update.go | 2 +- 4 files changed, 50 insertions(+), 629 deletions(-) diff --git a/api/client/secrets/client.go b/api/client/secrets/client.go index d73a0881d57..89330779c05 100644 --- a/api/client/secrets/client.go +++ b/api/client/secrets/client.go @@ -5,6 +5,7 @@ package secrets import ( "fmt" + "github.com/juju/errors" "github.com/juju/juju/api/base" diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index d6349aa4aff..6d78a1857fb 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -39889,18 +39889,6 @@ "Schema": { "type": "object", "properties": { - "CreateSecretURIs": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/CreateSecretURIsArg" - }, - "Result": { - "$ref": "#/definitions/StringResults" - } - }, - "description": "CreateSecretURIs creates new secret URIs." - }, "CreateSecrets": { "type": "object", "properties": { @@ -39913,63 +39901,6 @@ }, "description": "CreateSecrets creates new secrets." }, - "GetConsumerSecretsRevisionInfo": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/GetSecretConsumerInfoArgs" - }, - "Result": { - "$ref": "#/definitions/SecretConsumerInfoResults" - } - }, - "description": "GetConsumerSecretsRevisionInfo returns the latest secret revisions for the specified secrets.\nThis facade method is used for remote watcher to get the latest secret revisions and labels for a secret changed hook." - }, - "GetSecretBackendConfigs": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/SecretBackendArgs" - }, - "Result": { - "$ref": "#/definitions/SecretBackendConfigResults" - } - }, - "description": "GetSecretBackendConfigs gets the config needed to create a client to secret backends." - }, - "GetSecretContentInfo": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/GetSecretContentArgs" - }, - "Result": { - "$ref": "#/definitions/SecretContentResults" - } - }, - "description": "GetSecretContentInfo returns the secret values for the specified secrets." - }, - "GetSecretMetadata": { - "type": "object", - "properties": { - "Result": { - "$ref": "#/definitions/ListSecretResults" - } - }, - "description": "GetSecretMetadata returns metadata for the caller's secrets." - }, - "GetSecretRevisionContentInfo": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/SecretRevisionArg" - }, - "Result": { - "$ref": "#/definitions/SecretContentResults" - } - }, - "description": "GetSecretRevisionContentInfo returns the secret values for the specified secret revisions." - }, "GrantSecret": { "type": "object", "properties": { @@ -40018,42 +39949,6 @@ }, "description": "RevokeSecret revokes access to a user secret." }, - "SecretsGrant": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/GrantRevokeSecretArgs" - }, - "Result": { - "$ref": "#/definitions/ErrorResults" - } - }, - "description": "SecretsGrant grants access to a secret for the specified subjects." - }, - "SecretsRevoke": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/GrantRevokeSecretArgs" - }, - "Result": { - "$ref": "#/definitions/ErrorResults" - } - }, - "description": "SecretsRevoke revokes access to a secret for the specified subjects." - }, - "SecretsRotated": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/SecretRotatedArgs" - }, - "Result": { - "$ref": "#/definitions/ErrorResults" - } - }, - "description": "SecretsRotated records when secrets were last rotated." - }, "UpdateSecrets": { "type": "object", "properties": { @@ -40065,54 +39960,6 @@ } }, "description": "UpdateSecrets creates new secrets." - }, - "WatchConsumedSecretsChanges": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/Entities" - }, - "Result": { - "$ref": "#/definitions/StringsWatchResults" - } - }, - "description": "WatchConsumedSecretsChanges sets up a watcher to notify of changes to secret revisions for the specified consumers." - }, - "WatchObsolete": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/Entities" - }, - "Result": { - "$ref": "#/definitions/StringsWatchResult" - } - }, - "description": "WatchObsolete returns a watcher for notifying when:\n - a secret owned by the entity is deleted\n - a secret revision owed by the entity no longer\n has any consumers\n\nObsolete revisions results are \"uri/revno\" and deleted\nsecret results are \"uri\"." - }, - "WatchSecretRevisionsExpiryChanges": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/Entities" - }, - "Result": { - "$ref": "#/definitions/SecretTriggerWatchResult" - } - }, - "description": "WatchSecretRevisionsExpiryChanges sets up a watcher to notify of changes to secret revision expiry config." - }, - "WatchSecretsRotationChanges": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/Entities" - }, - "Result": { - "$ref": "#/definitions/SecretTriggerWatchResult" - } - }, - "description": "WatchSecretsRotationChanges sets up a watcher to notify of changes to secret rotation config." } }, "definitions": { @@ -40175,18 +40022,6 @@ "args" ] }, - "CreateSecretURIsArg": { - "type": "object", - "properties": { - "count": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "count" - ] - }, "DeleteSecretArg": { "type": "object", "properties": { @@ -40220,33 +40055,6 @@ "args" ] }, - "Entities": { - "type": "object", - "properties": { - "entities": { - "type": "array", - "items": { - "$ref": "#/definitions/Entity" - } - } - }, - "additionalProperties": false, - "required": [ - "entities" - ] - }, - "Entity": { - "type": "object", - "properties": { - "tag": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "tag" - ] - }, "Error": { "type": "object", "properties": { @@ -40296,170 +40104,73 @@ "results" ] }, - "GetSecretConsumerInfoArgs": { + "GrantRevokeUserSecretArg": { "type": "object", "properties": { - "consumer-tag": { - "type": "string" - }, - "uris": { + "applications": { "type": "array", "items": { "type": "string" } + }, + "uri": { + "type": "string" } }, "additionalProperties": false, "required": [ - "consumer-tag", - "uris" + "uri", + "applications" ] }, - "GetSecretContentArg": { + "ListSecretResult": { "type": "object", "properties": { + "create-time": { + "type": "string", + "format": "date-time" + }, + "description": { + "type": "string" + }, "label": { "type": "string" }, - "peek": { - "type": "boolean" + "latest-expire-time": { + "type": "string", + "format": "date-time" }, - "refresh": { - "type": "boolean" - }, - "uri": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri" - ] - }, - "GetSecretContentArgs": { - "type": "object", - "properties": { - "args": { - "type": "array", - "items": { - "$ref": "#/definitions/GetSecretContentArg" - } - } - }, - "additionalProperties": false, - "required": [ - "args" - ] - }, - "GrantRevokeSecretArg": { - "type": "object", - "properties": { - "role": { - "type": "string" - }, - "scope-tag": { - "type": "string" - }, - "subject-tags": { - "type": "array", - "items": { - "type": "string" - } + "latest-revision": { + "type": "integer" + }, + "next-rotate-time": { + "type": "string", + "format": "date-time" + }, + "owner-tag": { + "type": "string" + }, + "revisions": { + "type": "array", + "items": { + "$ref": "#/definitions/SecretRevision" + } + }, + "rotate-policy": { + "type": "string" + }, + "update-time": { + "type": "string", + "format": "date-time" }, "uri": { "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri", - "scope-tag", - "subject-tags", - "role" - ] - }, - "GrantRevokeSecretArgs": { - "type": "object", - "properties": { - "args": { - "type": "array", - "items": { - "$ref": "#/definitions/GrantRevokeSecretArg" - } - } - }, - "additionalProperties": false, - "required": [ - "args" - ] - }, - "GrantRevokeUserSecretArg": { - "type": "object", - "properties": { - "applications": { - "type": "array", - "items": { - "type": "string" - } - }, - "uri": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri", - "applications" - ] - }, - "ListSecretResult": { - "type": "object", - "properties": { - "create-time": { - "type": "string", - "format": "date-time" - }, - "description": { - "type": "string" - }, - "label": { - "type": "string" - }, - "latest-expire-time": { - "type": "string", - "format": "date-time" - }, - "latest-revision": { - "type": "integer" - }, - "next-rotate-time": { - "type": "string", - "format": "date-time" - }, - "owner-tag": { - "type": "string" - }, - "revisions": { - "type": "array", - "items": { - "$ref": "#/definitions/SecretRevision" - } - }, - "rotate-policy": { - "type": "string" - }, - "update-time": { - "type": "string", - "format": "date-time" - }, - "uri": { - "type": "string" - }, - "value": { - "$ref": "#/definitions/SecretValueResult" - }, - "version": { - "type": "integer" + }, + "value": { + "$ref": "#/definitions/SecretValueResult" + }, + "version": { + "type": "integer" } }, "additionalProperties": false, @@ -40504,127 +40215,6 @@ "filter" ] }, - "SecretBackendArgs": { - "type": "object", - "properties": { - "backend-ids": { - "type": "array", - "items": { - "type": "string" - } - }, - "for-drain": { - "type": "boolean" - } - }, - "additionalProperties": false, - "required": [ - "for-drain", - "backend-ids" - ] - }, - "SecretBackendConfig": { - "type": "object", - "properties": { - "params": { - "type": "object", - "patternProperties": { - ".*": { - "type": "object", - "additionalProperties": true - } - } - }, - "type": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "type" - ] - }, - "SecretBackendConfigResult": { - "type": "object", - "properties": { - "config": { - "$ref": "#/definitions/SecretBackendConfig" - }, - "draining": { - "type": "boolean" - }, - "model-controller": { - "type": "string" - }, - "model-name": { - "type": "string" - }, - "model-uuid": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "model-controller", - "model-uuid", - "model-name", - "draining" - ] - }, - "SecretBackendConfigResults": { - "type": "object", - "properties": { - "active-id": { - "type": "string" - }, - "results": { - "type": "object", - "patternProperties": { - ".*": { - "$ref": "#/definitions/SecretBackendConfigResult" - } - } - } - }, - "additionalProperties": false, - "required": [ - "active-id" - ] - }, - "SecretConsumerInfoResult": { - "type": "object", - "properties": { - "error": { - "$ref": "#/definitions/Error" - }, - "label": { - "type": "string" - }, - "revision": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "revision", - "label" - ] - }, - "SecretConsumerInfoResults": { - "type": "object", - "properties": { - "results": { - "type": "array", - "items": { - "$ref": "#/definitions/SecretConsumerInfoResult" - } - } - }, - "additionalProperties": false, - "required": [ - "results" - ] - }, "SecretContentParams": { "type": "object", "properties": { @@ -40642,42 +40232,6 @@ }, "additionalProperties": false }, - "SecretContentResult": { - "type": "object", - "properties": { - "backend-config": { - "$ref": "#/definitions/SecretBackendConfigResult" - }, - "content": { - "$ref": "#/definitions/SecretContentParams" - }, - "error": { - "$ref": "#/definitions/Error" - }, - "latest-revision": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "content" - ] - }, - "SecretContentResults": { - "type": "object", - "properties": { - "results": { - "type": "array", - "items": { - "$ref": "#/definitions/SecretContentResult" - } - } - }, - "additionalProperties": false, - "required": [ - "results" - ] - }, "SecretRevision": { "type": "object", "properties": { @@ -40708,106 +40262,6 @@ "revision" ] }, - "SecretRevisionArg": { - "type": "object", - "properties": { - "pending-delete": { - "type": "boolean" - }, - "revisions": { - "type": "array", - "items": { - "type": "integer" - } - }, - "uri": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri", - "revisions", - "pending-delete" - ] - }, - "SecretRotatedArg": { - "type": "object", - "properties": { - "original-revision": { - "type": "integer" - }, - "skip": { - "type": "boolean" - }, - "uri": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri", - "original-revision", - "skip" - ] - }, - "SecretRotatedArgs": { - "type": "object", - "properties": { - "args": { - "type": "array", - "items": { - "$ref": "#/definitions/SecretRotatedArg" - } - } - }, - "additionalProperties": false, - "required": [ - "args" - ] - }, - "SecretTriggerChange": { - "type": "object", - "properties": { - "next-trigger-time": { - "type": "string", - "format": "date-time" - }, - "revision": { - "type": "integer" - }, - "uri": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "uri", - "next-trigger-time" - ] - }, - "SecretTriggerWatchResult": { - "type": "object", - "properties": { - "changes": { - "type": "array", - "items": { - "$ref": "#/definitions/SecretTriggerChange" - } - }, - "error": { - "$ref": "#/definitions/Error" - }, - "watcher-id": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "watcher-id", - "changes" - ] - }, "SecretValueRef": { "type": "object", "properties": { @@ -40886,42 +40340,6 @@ "results" ] }, - "StringsWatchResult": { - "type": "object", - "properties": { - "changes": { - "type": "array", - "items": { - "type": "string" - } - }, - "error": { - "$ref": "#/definitions/Error" - }, - "watcher-id": { - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "watcher-id" - ] - }, - "StringsWatchResults": { - "type": "object", - "properties": { - "results": { - "type": "array", - "items": { - "$ref": "#/definitions/StringsWatchResult" - } - } - }, - "additionalProperties": false, - "required": [ - "results" - ] - }, "UpdateUserSecretArg": { "type": "object", "properties": { diff --git a/cmd/juju/commands/main_test.go b/cmd/juju/commands/main_test.go index 1bacd30d2ed..7305031c315 100644 --- a/cmd/juju/commands/main_test.go +++ b/cmd/juju/commands/main_test.go @@ -423,6 +423,7 @@ var commandNames = []string{ "remove-relation", "remove-saas", "remove-secret-backend", + "remove-secret", "remove-space", "remove-ssh-key", "remove-storage", @@ -488,6 +489,7 @@ var commandNames = []string{ "update-credential", "update-credentials", "update-secret-backend", + "update-secret", "update-storage-pool", "upgrade-controller", "upgrade-model", diff --git a/cmd/juju/secrets/update.go b/cmd/juju/secrets/update.go index dc42bf0629c..e48ca6d4605 100644 --- a/cmd/juju/secrets/update.go +++ b/cmd/juju/secrets/update.go @@ -76,7 +76,7 @@ Examples: return jujucmd.Info(&cmd.Info{ Name: "update-secret", Args: " [key[#base64|#file]=value...]", - Purpose: "Update an exisitng secret.", + Purpose: "Update an existing secret.", Doc: doc, }) } From 74520ab9ddcf43ce5af76bf415a6b73dc08c2a1b Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Mon, 14 Aug 2023 17:55:14 +1000 Subject: [PATCH 08/10] Add more tests for UpdateSecret and RemoveSecret; --- api/client/secrets/client.go | 4 +- api/client/secrets/client_test.go | 47 +++++++++++++++++++ apiserver/common/secrets/secrets.go | 1 - .../facades/agent/secretsmanager/register.go | 13 ++--- .../agent/secretsmanager/secrets_test.go | 4 -- apiserver/facades/client/secrets/secrets.go | 6 +-- cmd/juju/secrets/remove.go | 3 +- cmd/juju/secrets/remove_test.go | 15 +++++- cmd/juju/secrets/update.go | 7 ++- cmd/juju/secrets/update_test.go | 16 ++++++- rpc/params/secrets.go | 9 ++++ 11 files changed, 99 insertions(+), 26 deletions(-) diff --git a/api/client/secrets/client.go b/api/client/secrets/client.go index 89330779c05..627f85ac7ad 100644 --- a/api/client/secrets/client.go +++ b/api/client/secrets/client.go @@ -4,8 +4,6 @@ package secrets import ( - "fmt" - "github.com/juju/errors" "github.com/juju/juju/api/base" @@ -128,6 +126,7 @@ func (c *Client) CreateSecret(label, description string, data map[string]string) return result.Result, nil } +// UpdateSecret updates an existing secret. func (c *Client) UpdateSecret( uri *secrets.URI, autoPrune *bool, label, description string, data map[string]string, @@ -149,7 +148,6 @@ func (c *Client) UpdateSecret( if description != "" { arg.UpsertSecretArg.Description = &description } - fmt.Println(arg) err := c.facade.FacadeCall("UpdateSecrets", params.UpdateUserSecretArgs{Args: []params.UpdateUserSecretArg{arg}}, &results) if err != nil { return errors.Trace(err) diff --git a/api/client/secrets/client_test.go b/api/client/secrets/client_test.go index 02f567adaf7..5256923efa6 100644 --- a/api/client/secrets/client_test.go +++ b/api/client/secrets/client_test.go @@ -186,6 +186,32 @@ func (s *SecretsSuite) TestUpdateSecretError(c *gc.C) { c.Assert(err, gc.ErrorMatches, "user secrets not supported") } +func (s *SecretsSuite) TestUpdateSecretWithoutContent(c *gc.C) { + uri := secrets.NewURI() + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + c.Assert(objType, gc.Equals, "Secrets") + c.Assert(request, gc.Equals, "UpdateSecrets") + c.Assert(arg, gc.DeepEquals, params.UpdateUserSecretArgs{ + Args: []params.UpdateUserSecretArg{ + { + URI: uri.String(), + AutoPrune: ptr(true), + UpsertSecretArg: params.UpsertSecretArg{ + Label: ptr("label"), + Description: ptr("this is a secret."), + }, + }, + }, + }) + *(result.(*params.ErrorResults)) = params.ErrorResults{Results: []params.ErrorResult{{}}} + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 2} + client := apisecrets.NewClient(caller) + err := client.UpdateSecret(uri, ptr(true), "label", "this is a secret.", nil) + c.Assert(err, jc.ErrorIsNil) +} + func (s *SecretsSuite) TestUpdateSecret(c *gc.C) { uri := secrets.NewURI() apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { @@ -225,6 +251,27 @@ func (s *SecretsSuite) TestRemoveSecretError(c *gc.C) { } func (s *SecretsSuite) TestRemoveSecret(c *gc.C) { + uri := secrets.NewURI() + apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { + c.Assert(objType, gc.Equals, "Secrets") + c.Assert(request, gc.Equals, "RemoveSecrets") + c.Assert(arg, gc.DeepEquals, params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{ + {URI: uri.String()}, + }, + }) + *(result.(*params.ErrorResults)) = params.ErrorResults{ + Results: []params.ErrorResult{{}}, + } + return nil + }) + caller := testing.BestVersionCaller{apiCaller, 2} + client := apisecrets.NewClient(caller) + err := client.RemoveSecret(uri, nil) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *SecretsSuite) TestRemoveSecretWithRevision(c *gc.C) { uri := secrets.NewURI() apiCaller := testing.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error { c.Assert(objType, gc.Equals, "Secrets") diff --git a/apiserver/common/secrets/secrets.go b/apiserver/common/secrets/secrets.go index eeab5d967b4..25b23ccf39e 100644 --- a/apiserver/common/secrets/secrets.go +++ b/apiserver/common/secrets/secrets.go @@ -599,7 +599,6 @@ func RemoveSecrets( } } } - logger.Criticalf("externalRevisions: %#v", externalRevisions) if len(externalRevisions) == 0 { return result, nil } diff --git a/apiserver/facades/agent/secretsmanager/register.go b/apiserver/facades/agent/secretsmanager/register.go index 93ab59c62b0..a763d1055e5 100644 --- a/apiserver/facades/agent/secretsmanager/register.go +++ b/apiserver/facades/agent/secretsmanager/register.go @@ -14,8 +14,8 @@ import ( "github.com/juju/juju/api/controller/crossmodelsecrets" "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/common/secrets" + apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" - "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" "github.com/juju/juju/secrets/provider" @@ -43,17 +43,10 @@ func NewSecretManagerAPIV1(context facade.Context) (*SecretsManagerAPIV1, error) return &SecretsManagerAPIV1{SecretsManagerAPI: api}, nil } -func checkPermission(context facade.Context) error { - if context.Auth().AuthUnitAgent() || context.Auth().AuthApplicationAgent() { - return nil - } - return context.Auth().HasPermission(permission.WriteAccess, names.NewModelTag(context.State().ModelUUID())) -} - // NewSecretManagerAPI creates a SecretsManagerAPI. func NewSecretManagerAPI(context facade.Context) (*SecretsManagerAPI, error) { - if err := checkPermission(context); err != nil { - return nil, errors.Trace(err) + if !context.Auth().AuthUnitAgent() && !context.Auth().AuthApplicationAgent() { + return nil, apiservererrors.ErrPerm } model, err := context.State().Model() if err != nil { diff --git a/apiserver/facades/agent/secretsmanager/secrets_test.go b/apiserver/facades/agent/secretsmanager/secrets_test.go index 5bcd15f8777..de194386595 100644 --- a/apiserver/facades/agent/secretsmanager/secrets_test.go +++ b/apiserver/facades/agent/secretsmanager/secrets_test.go @@ -20,11 +20,9 @@ import ( apitesting "github.com/juju/juju/api/testing" commonsecrets "github.com/juju/juju/apiserver/common/secrets" - apiservererrors "github.com/juju/juju/apiserver/errors" facademocks "github.com/juju/juju/apiserver/facade/mocks" "github.com/juju/juju/apiserver/facades/agent/secretsmanager" "github.com/juju/juju/apiserver/facades/agent/secretsmanager/mocks" - "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" corewatcher "github.com/juju/juju/core/watcher" "github.com/juju/juju/rpc/params" @@ -468,7 +466,6 @@ func (s *SecretsManagerSuite) TestRemoveSecrets(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri - s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(apiservererrors.ErrPerm) s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ BackendID: "backend-id", @@ -508,7 +505,6 @@ func (s *SecretsManagerSuite) TestRemoveSecretRevision(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri - s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(apiservererrors.ErrPerm) s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return(nil, nil) s.leadership.EXPECT().LeadershipCheck("mariadb", "mariadb/0").Return(s.token) diff --git a/apiserver/facades/client/secrets/secrets.go b/apiserver/facades/client/secrets/secrets.go index 2e4e0b3d421..5c018277450 100644 --- a/apiserver/facades/client/secrets/secrets.go +++ b/apiserver/facades/client/secrets/secrets.go @@ -375,14 +375,14 @@ func (s *SecretsAPI) UpdateSecrets(args params.UpdateUserSecretArgs) (params.Err } func (s *SecretsAPI) updateSecret(backend provider.SecretsBackend, arg params.UpdateUserSecretArg) (errOut error) { + if err := arg.Validate(); err != nil { + return errors.Trace(err) + } uri, err := coresecrets.ParseURI(arg.URI) if err != nil { return errors.Trace(err) } - if arg.AutoPrune == nil && arg.Description == nil && arg.Label == nil && len(arg.Content.Data) == 0 { - return errors.New("at least one attribute to update must be specified") - } md, err := s.secretsState.GetSecret(uri) if err != nil { // Check if the uri exists or not. diff --git a/cmd/juju/secrets/remove.go b/cmd/juju/secrets/remove.go index 997125aa87a..c9611f2a050 100644 --- a/cmd/juju/secrets/remove.go +++ b/cmd/juju/secrets/remove.go @@ -51,7 +51,8 @@ Remove all the revisions of a secret with the specified URI or remove the provid Examples: remove-secret secret:9m4e2mr0ui3e8a215n4g - remove-secret secret:9m4e2mr0ui3e8a215n4g --revision 4 + + remove-secret secret:9m4e2mr0ui3e8a215n4g --revision 4 ` return jujucmd.Info(&cmd.Info{ Name: "remove-secret", diff --git a/cmd/juju/secrets/remove_test.go b/cmd/juju/secrets/remove_test.go index 12d726ae6a5..5fc33b35e4a 100644 --- a/cmd/juju/secrets/remove_test.go +++ b/cmd/juju/secrets/remove_test.go @@ -38,14 +38,14 @@ func (s *removeSuite) setup(c *gc.C) *gomock.Controller { return ctrl } -func (s *removeSuite) TestRemoveMissArg(c *gc.C) { +func (s *removeSuite) TestRemoveMissingArg(c *gc.C) { defer s.setup(c).Finish() _, err := cmdtesting.RunCommand(c, secrets.NewRemoveCommandForTest(s.store, s.secretsAPI), "--revision", "4") c.Assert(err, gc.ErrorMatches, `missing secret URI`) } -func (s *removeSuite) TestRemove(c *gc.C) { +func (s *removeSuite) TestRemoveWithRevision(c *gc.C) { defer s.setup(c).Finish() uri := coresecrets.NewURI() @@ -55,3 +55,14 @@ func (s *removeSuite) TestRemove(c *gc.C) { _, err := cmdtesting.RunCommand(c, secrets.NewRemoveCommandForTest(s.store, s.secretsAPI), uri.String(), "--revision", "4") c.Assert(err, jc.ErrorIsNil) } + +func (s *removeSuite) TestRemove(c *gc.C) { + defer s.setup(c).Finish() + + uri := coresecrets.NewURI() + s.secretsAPI.EXPECT().RemoveSecret(uri, nil).Return(nil) + s.secretsAPI.EXPECT().Close().Return(nil) + + _, err := cmdtesting.RunCommand(c, secrets.NewRemoveCommandForTest(s.store, s.secretsAPI), uri.String()) + c.Assert(err, jc.ErrorIsNil) +} diff --git a/cmd/juju/secrets/update.go b/cmd/juju/secrets/update.go index e48ca6d4605..38141f8d824 100644 --- a/cmd/juju/secrets/update.go +++ b/cmd/juju/secrets/update.go @@ -62,13 +62,18 @@ automatically removing secret content might result in data loss. Examples: update-secret secret:9m4e2mr0ui3e8a215n4g token=34ae35facd4 - update-secret secret:9m4e2mr0ui3e8a215n4g token=34ae35facd4 --auto-prune + update-secret secret:9m4e2mr0ui3e8a215n4g key#base64 AA== + + update-secret secret:9m4e2mr0ui3e8a215n4g token=34ae35facd4 --auto-prune + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ --info "my database password" \ data#base64 s3cret== + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ --info "my database password" + update-secret secret:9m4e2mr0ui3e8a215n4g --label db-password \ --info "my database password" \ --file=/path/to/file diff --git a/cmd/juju/secrets/update_test.go b/cmd/juju/secrets/update_test.go index 9aec9c4b549..db8e721f7d4 100644 --- a/cmd/juju/secrets/update_test.go +++ b/cmd/juju/secrets/update_test.go @@ -41,13 +41,27 @@ func (s *updateSuite) setup(c *gc.C) *gomock.Controller { return ctrl } -func (s *updateSuite) TestUpdateMissArg(c *gc.C) { +func (s *updateSuite) TestUpdateMissingArg(c *gc.C) { defer s.setup(c).Finish() _, err := cmdtesting.RunCommand(c, secrets.NewUpdateCommandForTest(s.store, s.secretsAPI), "--label", "label", "--info", "this is a secret.") c.Assert(err, gc.ErrorMatches, `missing secret URI`) } +func (s *updateSuite) TestUpdateWithoutContent(c *gc.C) { + defer s.setup(c).Finish() + + uri := coresecrets.NewURI() + s.secretsAPI.EXPECT().UpdateSecret(uri, ptr(true), "label", "this is a secret.", map[string]string{}).Return(nil) + s.secretsAPI.EXPECT().Close().Return(nil) + + _, err := cmdtesting.RunCommand(c, secrets.NewUpdateCommandForTest( + s.store, s.secretsAPI), uri.String(), + "--auto-prune", "--label", "label", "--info", "this is a secret.", + ) + c.Assert(err, jc.ErrorIsNil) +} + func (s *updateSuite) TestUpdateFromArg(c *gc.C) { defer s.setup(c).Finish() diff --git a/rpc/params/secrets.go b/rpc/params/secrets.go index c5775bfd1e9..bb848bfd464 100644 --- a/rpc/params/secrets.go +++ b/rpc/params/secrets.go @@ -7,6 +7,7 @@ import ( "time" "github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery" + "github.com/juju/errors" "gopkg.in/macaroon.v2" "github.com/juju/juju/core/secrets" @@ -127,6 +128,14 @@ type UpdateUserSecretArg struct { AutoPrune *bool `json:"auto-prune,omitempty"` } +// Validate validates the UpdateUserSecretArg. +func (arg UpdateUserSecretArg) Validate() error { + if arg.AutoPrune == nil && arg.Description == nil && arg.Label == nil && len(arg.Content.Data) == 0 { + return errors.New("at least one attribute to update must be specified") + } + return nil +} + // DeleteSecretArgs holds args for deleting secrets. type DeleteSecretArgs struct { Args []DeleteSecretArg `json:"args"` From 4158c7c9419a8815b1198bd184a6d25f1d53e71b Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Wed, 9 Aug 2023 11:58:56 +0100 Subject: [PATCH 09/10] Remove unsupported series from core Most notably, drop OpenSUSE and MacOSX, which despite being listed as supported series, are not supported and never have been Also drop EoL CentOS 8. Keep Centos7 which is EoL in June 2024 and CentOS9 which is recieving rolling updates --- cloudconfig/cloudinit/cloudinit_opensuse.go | 34 ------- cloudconfig/cloudinit/interface.go | 11 --- cloudconfig/machinecloudconfig.go | 2 +- cloudconfig/machinecloudconfig_test.go | 6 -- cloudconfig/userdatacfg.go | 4 - cloudconfig/userdatacfg_test.go | 23 ----- cloudconfig/userdatacfg_unix.go | 19 +--- cmd/juju/application/deploy_test.go | 8 +- .../deployer/bundlehandler_test.go | 2 +- .../application/deployer/deployer_test.go | 2 +- cmd/juju/application/export_test.go | 2 +- cmd/juju/application/refresh_test.go | 4 +- cmd/juju/application/unexpose_test.go | 2 +- cmd/juju/machine/upgrademachine_test.go | 2 +- container/lxd/image.go | 4 - core/base/supported.go | 99 +------------------ core/base/supportedbases_test.go | 4 - core/base/supportedseries.go | 12 --- core/base/supportedseries_linux_test.go | 4 +- core/base/supportedseries_test.go | 14 +-- core/os/os.go | 7 +- core/os/os_linux.go | 2 - core/os/os_test.go | 17 +--- core/paths/logfile.go | 2 - .../manual/sshprovisioner/sshprovisioner.go | 5 - packaging/dependency/kvm.go | 3 +- packaging/dependency/lxd.go | 5 +- packaging/dependency/mongo.go | 2 +- packaging/manager.go | 11 +-- packaging/manager_test.go | 22 +---- provider/common/disk.go | 11 +-- provider/lxd/userdata.go | 2 +- tests/suites/deploy/os.sh | 22 ++--- version/current_test.go | 25 +---- worker/uniter/runner/context/env.go | 23 ----- worker/uniter/runner/context/env_test.go | 61 ------------ 36 files changed, 60 insertions(+), 418 deletions(-) delete mode 100644 cloudconfig/cloudinit/cloudinit_opensuse.go diff --git a/cloudconfig/cloudinit/cloudinit_opensuse.go b/cloudconfig/cloudinit/cloudinit_opensuse.go deleted file mode 100644 index 47c46ec2a1c..00000000000 --- a/cloudconfig/cloudinit/cloudinit_opensuse.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2015 Canonical Ltd. -// Copyright 2015 Cloudbase Solutions SRL -// Licensed under the AGPLv3, see LICENCE file for details. - -package cloudinit - -import ( - "github.com/juju/packaging/v2/commands" - "github.com/juju/proxy" -) - -// Implementation of PackageHelper for OpenSUSE -type openSUSEHelper struct { - paccmder commands.PackageCommander -} - -// Returns the list of required packages in OpenSUSE -func (helper openSUSEHelper) getRequiredPackages() []string { - return []string{ - "curl", - "bridge-utils", - //"cloud-utils", Put as a requirement to the cloud image (requires subscription) - "ncat", - "tmux", - } -} - -// addPackageProxyCmd is a helper method which returns the corresponding runcmd -// to apply the package proxy settings for OpenSUSE -func (helper openSUSEHelper) addPackageProxyCmd(url string) string { - return helper.paccmder.SetProxyCmds(proxy.Settings{ - Http: url, - })[0] -} diff --git a/cloudconfig/cloudinit/interface.go b/cloudconfig/cloudinit/interface.go index 5949ee4f6bb..32f5aae6f6a 100644 --- a/cloudconfig/cloudinit/interface.go +++ b/cloudconfig/cloudinit/interface.go @@ -447,17 +447,6 @@ func New(osname string, opts ...func(*cloudConfig)) (CloudConfig, error) { cloudConfig: cfg, helper: centOSHelper{}, }, nil - case os.OpenSUSE: - cfg.paccmder = map[jujupackaging.PackageManagerName]commands.PackageCommander{ - jujupackaging.ZypperPackageManager: commands.NewZypperPackageCommander(), - } - cfg.pacconfer = map[jujupackaging.PackageManagerName]config.PackagingConfigurer{ - jujupackaging.ZypperPackageManager: config.NewZypperPackagingConfigurer(osname), - } - return ¢OSCloudConfig{ - cloudConfig: cfg, - helper: openSUSEHelper{paccmder: commands.NewZypperPackageCommander()}, - }, nil default: return nil, errors.NotFoundf("cloudconfig for os %q", osname) } diff --git a/cloudconfig/machinecloudconfig.go b/cloudconfig/machinecloudconfig.go index 0bf0d775784..6cf60ed8f7a 100644 --- a/cloudconfig/machinecloudconfig.go +++ b/cloudconfig/machinecloudconfig.go @@ -81,7 +81,7 @@ func NewMachineInitReaderFromConfig(cfg MachineInitReaderConfig) InitReader { // machine. It is sourced from both Cloud-Init and Curtin data. func (r *MachineInitReader) GetInitConfig() (map[string]interface{}, error) { switch utilsos.OSTypeForName(r.config.Base.OS) { - case utilsos.Ubuntu, utilsos.CentOS, utilsos.OpenSUSE: + case utilsos.Ubuntu, utilsos.CentOS: hostSeries, err := osseries.HostSeries() series, err2 := corebase.GetSeriesFromBase(r.config.Base) if err != nil || err2 != nil || series != hostSeries { diff --git a/cloudconfig/machinecloudconfig_test.go b/cloudconfig/machinecloudconfig_test.go index a9841fccdda..de815f0f706 100644 --- a/cloudconfig/machinecloudconfig_test.go +++ b/cloudconfig/machinecloudconfig_test.go @@ -83,12 +83,6 @@ var cloudinitDataVerifyTests = []cloudinitDataVerifyTest{ containerBase: corebase.MakeDefaultBase("centos", "7"), result: expectedResult, }, - { - description: "centos8 on centos8", - machineBase: corebase.MakeDefaultBase("centos", "8"), - containerBase: corebase.MakeDefaultBase("centos", "8"), - result: expectedResult, - }, } func (s *fromHostSuite) TestGetMachineCloudInitDataVerifySeries(c *gc.C) { diff --git a/cloudconfig/userdatacfg.go b/cloudconfig/userdatacfg.go index 031023b9355..ad76f9c4f07 100644 --- a/cloudconfig/userdatacfg.go +++ b/cloudconfig/userdatacfg.go @@ -72,8 +72,6 @@ func NewUserdataConfig(icfg *instancecfg.InstanceConfig, conf cloudinit.CloudCon return &unixConfigure{base}, nil case os.CentOS: return &unixConfigure{base}, nil - case os.OpenSUSE: - return &unixConfigure{base}, nil default: return nil, errors.NotSupportedf("OS %s", icfg.Base.OS) } @@ -143,8 +141,6 @@ func SetUbuntuUser(conf cloudinit.CloudConfig, authorizedKeys string) { groups = UbuntuGroups case os.CentOS: groups = CentOSGroups - case os.OpenSUSE: - groups = OpenSUSEGroups } conf.AddUser(&cloudinit.User{ Name: "ubuntu", diff --git a/cloudconfig/userdatacfg_test.go b/cloudconfig/userdatacfg_test.go index 8f7f5e38aa4..67b45242df3 100644 --- a/cloudconfig/userdatacfg_test.go +++ b/cloudconfig/userdatacfg_test.go @@ -429,18 +429,6 @@ systemctl is-active firewalld &> /dev/null && systemctl stop firewalld || true sed -i "s/\^\.\*requiretty/#Defaults requiretty/" /etc/sudoers `, }, - // OpenSUSE non controller - { - cfg: makeNormalConfig(corebase.MakeDefaultBase("opensuse", "opensuse42"), 0), - inexactMatch: true, - upgradedToVersion: "1.2.3", - expectScripts: ` -systemctl is-enabled firewalld &> /dev/null && systemctl mask firewalld || true -systemctl is-active firewalld &> /dev/null && systemctl stop firewalld || true -sed -i "s/\^\.\*requiretty/#Defaults requiretty/" /etc/sudoers -`, - }, - // check that it works ok with compound machine ids. { cfg: makeNormalConfig(jammy, 0).mutate(func(cfg *testInstanceConfig) { @@ -1418,14 +1406,3 @@ func (*cloudinitSuite) TestCloudInitBootstrapInitialSSHKeys(c *gc.C) { `ssh-keygen -t ed25519 -N "" -f /etc/ssh/ssh_host_ed25519_key || true`, }) } - -func (*cloudinitSuite) TestSetUbuntuUserOpenSUSE(c *gc.C) { - ci, err := cloudinit.New("opensuse") - c.Assert(err, jc.ErrorIsNil) - cloudconfig.SetUbuntuUser(ci, "akey\n#also\nbkey") - data, err := ci.RenderYAML() - c.Assert(err, jc.ErrorIsNil) - keys := []string{"akey", "bkey"} - expected := expectedUbuntuUser(cloudconfig.OpenSUSEGroups, keys) - c.Assert(string(data), jc.YAMLEquals, expected) -} diff --git a/cloudconfig/userdatacfg_unix.go b/cloudconfig/userdatacfg_unix.go index 78a67367756..a7517701842 100644 --- a/cloudconfig/userdatacfg_unix.go +++ b/cloudconfig/userdatacfg_unix.go @@ -125,10 +125,6 @@ var ( // CentOSGroups is the set of unix groups to add the "ubuntu" user to // when initializing a CentOS system. CentOSGroups = []string{"adm", "systemd-journal", "wheel"} - - // OpenSUSEGroups is the set of unix groups to add the "ubuntu" user to - // when initializing a OpenSUSE system. - OpenSUSEGroups = []string{"users"} ) type unixConfigure struct { @@ -187,19 +183,6 @@ func (w *unixConfigure) ConfigureBasic() error { `sed -i "s/^.*requiretty/#Defaults requiretty/" /etc/sudoers`, ) - case os.OpenSUSE: - w.conf.AddScripts( - // Mask and stop firewalld, if enabled, so it cannot start. See - // http://pad.lv/1492066. firewalld might be missing, in which case - // is-enabled and is-active prints an error, which is why the output - // is suppressed. - "systemctl is-enabled firewalld &> /dev/null && systemctl mask firewalld || true", - "systemctl is-active firewalld &> /dev/null && systemctl stop firewalld || true", - `sed -i "s/^.*requiretty/#Defaults requiretty/" /etc/sudoers`, - //Scripts assume ubuntu group for ubuntu user... - `(grep ubuntu /etc/group) || groupadd ubuntu`, - `usermod -g ubuntu -G ubuntu,users ubuntu`, - ) } SetUbuntuUser(w.conf, w.icfg.AuthorizedKeys) @@ -237,7 +220,7 @@ func (w *unixConfigure) ConfigureBasic() error { func (w *unixConfigure) setDataDirPermissions() string { var user string switch w.os { - case os.CentOS, os.OpenSUSE: + case os.CentOS: user = "root" default: user = "syslog" diff --git a/cmd/juju/application/deploy_test.go b/cmd/juju/application/deploy_test.go index fc205221990..048f3fb1deb 100644 --- a/cmd/juju/application/deploy_test.go +++ b/cmd/juju/application/deploy_test.go @@ -154,7 +154,7 @@ func (s *DeploySuiteBase) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "jammy", "xenial", ), nil }, @@ -1044,7 +1044,7 @@ func (s *CAASDeploySuiteBase) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "jammy", "xenial", ), nil }, @@ -1485,7 +1485,7 @@ func (s *DeploySuite) setupNonESMBase(c *gc.C) (corebase.Base, string) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "jammy", "xenial", ), nil }, @@ -1944,7 +1944,7 @@ func (s *DeployUnitTestSuite) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "jammy", "xenial", ), nil }, diff --git a/cmd/juju/application/deployer/bundlehandler_test.go b/cmd/juju/application/deployer/bundlehandler_test.go index 7c7213e13b0..b3b6a9d38f9 100644 --- a/cmd/juju/application/deployer/bundlehandler_test.go +++ b/cmd/juju/application/deployer/bundlehandler_test.go @@ -66,7 +66,7 @@ func (s *BundleDeployRepositorySuite) SetUpTest(_ *gc.C) { s.PatchValue(&SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/cmd/juju/application/deployer/deployer_test.go b/cmd/juju/application/deployer/deployer_test.go index 0b444de38b5..0bfba50eaf1 100644 --- a/cmd/juju/application/deployer/deployer_test.go +++ b/cmd/juju/application/deployer/deployer_test.go @@ -60,7 +60,7 @@ func (s *deployerSuite) SetUpTest(_ *gc.C) { s.PatchValue(&SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/cmd/juju/application/export_test.go b/cmd/juju/application/export_test.go index a0d17e2e133..82ed6a1d502 100644 --- a/cmd/juju/application/export_test.go +++ b/cmd/juju/application/export_test.go @@ -249,7 +249,7 @@ func (s *RepoSuiteBaseSuite) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/cmd/juju/application/refresh_test.go b/cmd/juju/application/refresh_test.go index d4938829ded..1cbd7d7d7d4 100644 --- a/cmd/juju/application/refresh_test.go +++ b/cmd/juju/application/refresh_test.go @@ -435,7 +435,7 @@ func (s *RefreshErrorsStateSuite) SetUpSuite(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, @@ -565,7 +565,7 @@ func (s *RefreshSuccessStateSuite) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/cmd/juju/application/unexpose_test.go b/cmd/juju/application/unexpose_test.go index 9f3b28285be..fb7de09e90e 100644 --- a/cmd/juju/application/unexpose_test.go +++ b/cmd/juju/application/unexpose_test.go @@ -36,7 +36,7 @@ func (s *UnexposeSuite) SetUpTest(c *gc.C) { s.PatchValue(&deployer.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/cmd/juju/machine/upgrademachine_test.go b/cmd/juju/machine/upgrademachine_test.go index 49581333ec7..0bbe4c1e136 100644 --- a/cmd/juju/machine/upgrademachine_test.go +++ b/cmd/juju/machine/upgrademachine_test.go @@ -72,7 +72,7 @@ func (s *UpgradeMachineSuite) SetUpTest(c *gc.C) { s.PatchValue(&machine.SupportedJujuSeries, func(time.Time, string, string) (set.Strings, error) { return set.NewStrings( - "centos7", "centos8", "centos9", "genericlinux", "kubernetes", "opensuseleap", + "centos7", "centos9", "genericlinux", "kubernetes", "jammy", "focal", "bionic", "xenial", ), nil }, diff --git a/container/lxd/image.go b/container/lxd/image.go index d2b15236003..55829250eb3 100644 --- a/container/lxd/image.go +++ b/container/lxd/image.go @@ -221,10 +221,6 @@ func constructBaseRemoteAlias(base jujubase.Base, arch string) (string, error) { return "centos/9-Stream/cloud/amd64", nil } } - case jujuos.OpenSUSE: - if base.Channel.Track == "opensuse42" && arch == jujuarch.AMD64 { - return "opensuse/42.2/amd64", nil - } } return "", errors.NotSupportedf("base %q", base.DisplayString()) } diff --git a/core/base/supported.go b/core/base/supported.go index 35150ee771b..332b93c2a1a 100644 --- a/core/base/supported.go +++ b/core/base/supported.go @@ -380,11 +380,9 @@ var ubuntuSeries = map[SeriesName]seriesVersion{ } const ( - Centos7 SeriesName = "centos7" - Centos8 SeriesName = "centos8" - Centos9 SeriesName = "centos9" - OpenSUSELeap SeriesName = "opensuseleap" - Kubernetes SeriesName = "kubernetes" + Centos7 SeriesName = "centos7" + Centos9 SeriesName = "centos9" + Kubernetes SeriesName = "kubernetes" ) var centosSeries = map[SeriesName]seriesVersion{ @@ -393,11 +391,6 @@ var centosSeries = map[SeriesName]seriesVersion{ Version: "7", Supported: true, }, - Centos8: { - WorkloadType: OtherWorkloadType, - Version: "8", - Supported: true, - }, Centos9: { WorkloadType: OtherWorkloadType, Version: "9", @@ -405,14 +398,6 @@ var centosSeries = map[SeriesName]seriesVersion{ }, } -var opensuseSeries = map[SeriesName]seriesVersion{ - OpenSUSELeap: { - WorkloadType: OtherWorkloadType, - Version: "opensuse42", - Supported: true, - }, -} - var kubernetesSeries = map[SeriesName]seriesVersion{ Kubernetes: { WorkloadType: OtherWorkloadType, @@ -420,81 +405,3 @@ var kubernetesSeries = map[SeriesName]seriesVersion{ Supported: true, }, } - -var macOSXSeries = map[SeriesName]seriesVersion{ - "catalina": { - WorkloadType: UnsupportedWorkloadType, - Version: "19", - Supported: true, - }, - "mojave": { - WorkloadType: UnsupportedWorkloadType, - Version: "18", - Supported: true, - }, - "highsierra": { - WorkloadType: UnsupportedWorkloadType, - Version: "17", - Supported: true, - }, - "sierra": { - WorkloadType: UnsupportedWorkloadType, - Version: "16", - Supported: true, - }, - "elcapitan": { - WorkloadType: UnsupportedWorkloadType, - Version: "15", - Supported: true, - }, - "yosemite": { - WorkloadType: UnsupportedWorkloadType, - Version: "14", - Supported: true, - }, - "mavericks": { - WorkloadType: UnsupportedWorkloadType, - Version: "13", - Supported: true, - }, - "mountainlion": { - WorkloadType: UnsupportedWorkloadType, - Version: "12", - Supported: true, - }, - "lion": { - WorkloadType: UnsupportedWorkloadType, - Version: "11", - Supported: true, - }, - "snowleopard": { - WorkloadType: UnsupportedWorkloadType, - Version: "10", - Supported: true, - }, - "leopard": { - WorkloadType: UnsupportedWorkloadType, - Version: "9", - Supported: true, - }, - "tiger": { - WorkloadType: UnsupportedWorkloadType, - Version: "8", - Supported: true, - }, - "panther": { - WorkloadType: UnsupportedWorkloadType, - Version: "7", - Supported: true, - }, - "jaguar": { - WorkloadType: UnsupportedWorkloadType, - Version: "6", - Supported: true, - }, - "puma": { - WorkloadType: UnsupportedWorkloadType, - Version: "5", - Supported: true, - }, -} diff --git a/core/base/supportedbases_test.go b/core/base/supportedbases_test.go index 74955187642..400bb21dbb6 100644 --- a/core/base/supportedbases_test.go +++ b/core/base/supportedbases_test.go @@ -30,11 +30,9 @@ func (s *BasesSuite) TestWorkloadBases(c *gc.C) { imageStream: Daily, expectedBase: []Base{ MustParseBaseFromString("centos@7/stable"), - MustParseBaseFromString("centos@8/stable"), MustParseBaseFromString("centos@9/stable"), MustParseBaseFromString("genericlinux@genericlinux/stable"), MustParseBaseFromString("kubernetes@kubernetes"), - MustParseBaseFromString("opensuse@opensuse42/stable"), MustParseBaseFromString("ubuntu@20.04/stable"), MustParseBaseFromString("ubuntu@22.04/stable"), }, @@ -44,11 +42,9 @@ func (s *BasesSuite) TestWorkloadBases(c *gc.C) { imageStream: Daily, expectedBase: []Base{ MustParseBaseFromString("centos@7/stable"), - MustParseBaseFromString("centos@8/stable"), MustParseBaseFromString("centos@9/stable"), MustParseBaseFromString("genericlinux@genericlinux/stable"), MustParseBaseFromString("kubernetes@kubernetes"), - MustParseBaseFromString("opensuse@opensuse42/stable"), MustParseBaseFromString("ubuntu@20.04/stable"), MustParseBaseFromString("ubuntu@22.04/stable"), }, diff --git a/core/base/supportedseries.go b/core/base/supportedseries.go index fa5d52cd133..4a4e2ae5814 100644 --- a/core/base/supportedseries.go +++ b/core/base/supportedseries.go @@ -175,15 +175,9 @@ func composeSeriesVersions() { for k, v := range ubuntuSeries { allSeriesVersions[k] = v } - for k, v := range macOSXSeries { - allSeriesVersions[k] = v - } for k, v := range centosSeries { allSeriesVersions[k] = v } - for k, v := range opensuseSeries { - allSeriesVersions[k] = v - } for k, v := range kubernetesSeries { allSeriesVersions[k] = v } @@ -261,15 +255,9 @@ func getOSFromSeries(series SeriesName) (coreos.OSType, error) { if _, ok := ubuntuSeries[series]; ok { return coreos.Ubuntu, nil } - if _, ok := macOSXSeries[series]; ok { - return coreos.OSX, nil - } if _, ok := centosSeries[series]; ok { return coreos.CentOS, nil } - if _, ok := opensuseSeries[series]; ok { - return coreos.OpenSUSE, nil - } if _, ok := kubernetesSeries[series]; ok { return coreos.Kubernetes, nil } diff --git a/core/base/supportedseries_linux_test.go b/core/base/supportedseries_linux_test.go index 26ce94484c5..cba30c450ab 100644 --- a/core/base/supportedseries_linux_test.go +++ b/core/base/supportedseries_linux_test.go @@ -80,6 +80,6 @@ func (s *SupportedSeriesLinuxSuite) TestWorkloadSeries(c *gc.C) { series, err := WorkloadSeries(time.Time{}, "", "") c.Assert(err, jc.ErrorIsNil) c.Assert(series.SortedValues(), gc.DeepEquals, []string{ - "centos7", "centos8", "centos9", "focal", "genericlinux", "jammy", "kubernetes", - "opensuseleap"}) + "centos7", "centos9", "focal", "genericlinux", "jammy", "kubernetes", + }) } diff --git a/core/base/supportedseries_test.go b/core/base/supportedseries_test.go index 8f4993d9645..f9f5ba7b2d1 100644 --- a/core/base/supportedseries_test.go +++ b/core/base/supportedseries_test.go @@ -39,7 +39,7 @@ func (s *SupportedSeriesSuite) TestSeriesForTypes(c *gc.C) { c.Assert(ctrlSeries, jc.DeepEquals, []string{"jammy", "focal"}) wrkSeries := info.workloadSeries(false) - c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos8", "centos7", "genericlinux", "kubernetes", "opensuseleap"}) + c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos7", "genericlinux", "kubernetes"}) } func (s *SupportedSeriesSuite) TestSeriesForTypesUsingImageStream(c *gc.C) { @@ -55,7 +55,7 @@ func (s *SupportedSeriesSuite) TestSeriesForTypesUsingImageStream(c *gc.C) { c.Assert(ctrlSeries, jc.DeepEquals, []string{"jammy", "focal"}) wrkSeries := info.workloadSeries(false) - c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos8", "centos7", "genericlinux", "kubernetes", "opensuseleap"}) + c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos7", "genericlinux", "kubernetes"}) } func (s *SupportedSeriesSuite) TestSeriesForTypesUsingInvalidImageStream(c *gc.C) { @@ -71,7 +71,7 @@ func (s *SupportedSeriesSuite) TestSeriesForTypesUsingInvalidImageStream(c *gc.C c.Assert(ctrlSeries, jc.DeepEquals, []string{"jammy", "focal"}) wrkSeries := info.workloadSeries(false) - c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos8", "centos7", "genericlinux", "kubernetes", "opensuseleap"}) + c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos7", "genericlinux", "kubernetes"}) } func (s *SupportedSeriesSuite) TestSeriesForTypesUsingInvalidSeries(c *gc.C) { @@ -87,7 +87,7 @@ func (s *SupportedSeriesSuite) TestSeriesForTypesUsingInvalidSeries(c *gc.C) { c.Assert(ctrlSeries, jc.DeepEquals, []string{"jammy", "focal"}) wrkSeries := info.workloadSeries(false) - c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos8", "centos7", "genericlinux", "kubernetes", "opensuseleap"}) + c.Assert(wrkSeries, jc.DeepEquals, []string{"jammy", "focal", "centos9", "centos7", "genericlinux", "kubernetes"}) } var getOSFromSeriesTests = []struct { @@ -97,15 +97,9 @@ var getOSFromSeriesTests = []struct { }{{ series: "precise", want: coreos.Ubuntu, -}, { - series: "mountainlion", - want: coreos.OSX, }, { series: "centos7", want: coreos.CentOS, -}, { - series: "opensuseleap", - want: coreos.OpenSUSE, }, { series: "kubernetes", want: coreos.Kubernetes, diff --git a/core/os/os.go b/core/os/os.go index 757d149757b..e458cb7f050 100644 --- a/core/os/os.go +++ b/core/os/os.go @@ -18,7 +18,6 @@ const ( OSX CentOS GenericLinux - OpenSUSE Kubernetes ) @@ -34,8 +33,6 @@ func (t OSType) String() string { return "CentOS" case GenericLinux: return "GenericLinux" - case OpenSUSE: - return "OpenSUSE" case Kubernetes: return "Kubernetes" } @@ -49,10 +46,8 @@ func init() { Unknown, Ubuntu, Windows, - OSX, CentOS, GenericLinux, - OpenSUSE, Kubernetes, } validOSTypeNames = make(map[string]OSType) @@ -103,7 +98,7 @@ func (t OSType) EquivalentTo(t2 OSType) bool { // IsLinux returns true if the OS type is a Linux variant. func (t OSType) IsLinux() bool { switch t { - case Ubuntu, CentOS, GenericLinux, OpenSUSE: + case Ubuntu, CentOS, GenericLinux: return true } return false diff --git a/core/os/os_linux.go b/core/os/os_linux.go index 765c8ad64bb..b50349928a0 100644 --- a/core/os/os_linux.go +++ b/core/os/os_linux.go @@ -39,8 +39,6 @@ func updateOS(f string) (OSType, error) { return Ubuntu, nil case strings.ToLower(CentOS.String()): return CentOS, nil - case strings.ToLower(OpenSUSE.String()): - return OpenSUSE, nil default: return GenericLinux, nil } diff --git a/core/os/os_test.go b/core/os/os_test.go index 91c90093b91..815bf35bacb 100644 --- a/core/os/os_test.go +++ b/core/os/os_test.go @@ -20,15 +20,13 @@ func (s *osSuite) TestHostOS(c *gc.C) { switch runtime.GOOS { case "windows": c.Assert(os, gc.Equals, Windows) - case "darwin": + case "OSX": c.Assert(os, gc.Equals, OSX) case "linux": // TODO(mjs) - this should really do more by patching out // osReleaseFile and testing the corner cases. switch os { case Ubuntu, CentOS, GenericLinux: - case OpenSUSE: - c.Assert(os, gc.Equals, OpenSUSE) default: c.Fatalf("unknown linux version: %v", os) } @@ -40,24 +38,19 @@ func (s *osSuite) TestHostOS(c *gc.C) { func (s *osSuite) TestEquivalentTo(c *gc.C) { c.Check(Ubuntu.EquivalentTo(CentOS), jc.IsTrue) c.Check(Ubuntu.EquivalentTo(GenericLinux), jc.IsTrue) - c.Check(Ubuntu.EquivalentTo(OpenSUSE), jc.IsTrue) c.Check(GenericLinux.EquivalentTo(Ubuntu), jc.IsTrue) - c.Check(GenericLinux.EquivalentTo(OpenSUSE), jc.IsTrue) c.Check(CentOS.EquivalentTo(CentOS), jc.IsTrue) - c.Check(CentOS.EquivalentTo(OpenSUSE), jc.IsTrue) - - c.Check(OSX.EquivalentTo(Ubuntu), jc.IsFalse) - c.Check(OSX.EquivalentTo(Windows), jc.IsFalse) - c.Check(GenericLinux.EquivalentTo(OSX), jc.IsFalse) } func (s *osSuite) TestIsLinux(c *gc.C) { c.Check(Ubuntu.IsLinux(), jc.IsTrue) c.Check(CentOS.IsLinux(), jc.IsTrue) c.Check(GenericLinux.IsLinux(), jc.IsTrue) - c.Check(OpenSUSE.IsLinux(), jc.IsTrue) - c.Check(OSX.IsLinux(), jc.IsFalse) c.Check(Windows.IsLinux(), jc.IsFalse) c.Check(Unknown.IsLinux(), jc.IsFalse) + + c.Check(OSX.EquivalentTo(Ubuntu), jc.IsFalse) + c.Check(OSX.EquivalentTo(Windows), jc.IsFalse) + c.Check(GenericLinux.EquivalentTo(OSX), jc.IsFalse) } diff --git a/core/paths/logfile.go b/core/paths/logfile.go index 959184606ce..83834f1b5b9 100644 --- a/core/paths/logfile.go +++ b/core/paths/logfile.go @@ -65,8 +65,6 @@ func SyslogUserGroup() (string, string) { switch jujuos.HostOS() { case jujuos.CentOS: return "root", "adm" - case jujuos.OpenSUSE: - return "root", "root" default: return "syslog", "adm" } diff --git a/environs/manual/sshprovisioner/sshprovisioner.go b/environs/manual/sshprovisioner/sshprovisioner.go index e7bfc85af45..adfa4ded7f4 100644 --- a/environs/manual/sshprovisioner/sshprovisioner.go +++ b/environs/manual/sshprovisioner/sshprovisioner.go @@ -202,11 +202,6 @@ os_id=$(grep '^ID=' /etc/os-release | tr -d '"' | cut -d= -f2) if [ "$os_id" = 'centos' ]; then os_version=$(grep '^VERSION_ID=' /etc/os-release | tr -d '"' | cut -d= -f2) echo "centos$os_version" -elif [ "$os_id" = 'opensuse' ]; then - os_version=$(grep '^VERSION_ID=' /etc/os-release | tr -d '"' | cut -d= -f2 | cut -d. -f1) - if [ $os_version -eq 42 ]; then - echo "opensuseleap" - fi else lsb_release -cs fi diff --git a/packaging/dependency/kvm.go b/packaging/dependency/kvm.go index 9a3442f057e..f224fff7a48 100644 --- a/packaging/dependency/kvm.go +++ b/packaging/dependency/kvm.go @@ -7,6 +7,7 @@ import ( "github.com/juju/errors" "github.com/juju/utils/v3/arch" + "github.com/juju/juju/core/base" "github.com/juju/juju/packaging" ) @@ -21,7 +22,7 @@ type kvmDependency struct { // PackageList implements packaging.Dependency. func (dep kvmDependency) PackageList(series string) ([]packaging.Package, error) { - if series == "centos7" || series == "centos8" || series == "centos9" || series == "opensuseleap" { + if series == base.Centos7.String() || series == base.Centos9.String() { return nil, errors.NotSupportedf("installing kvm on series %q", series) } diff --git a/packaging/dependency/lxd.go b/packaging/dependency/lxd.go index 9569a623a58..698de312404 100644 --- a/packaging/dependency/lxd.go +++ b/packaging/dependency/lxd.go @@ -8,6 +8,7 @@ import ( "github.com/juju/errors" + "github.com/juju/juju/core/base" "github.com/juju/juju/packaging" ) @@ -30,9 +31,9 @@ func (dep lxdDependency) PackageList(series string) ([]packaging.Package, error) var pkg packaging.Package switch series { - case "centos7", "centos8", "centos9", "opensuseleap": + case base.Centos7.String(), base.Centos9.String(): return nil, errors.NotSupportedf("LXD containers on series %q", series) - case "bionic", blankSeries: + case base.Bionic.String(), blankSeries: pkg.Name = "lxd" pkg.PackageManager = packaging.AptPackageManager default: // Use snaps for cosmic and beyond diff --git a/packaging/dependency/mongo.go b/packaging/dependency/mongo.go index 14c91c8cdd6..8f2eeeaf8e2 100644 --- a/packaging/dependency/mongo.go +++ b/packaging/dependency/mongo.go @@ -31,7 +31,7 @@ func (dep mongoDependency) PackageList(series string) ([]packaging.Package, erro ) switch series { - case "centos7", "centos8", "centos9", "opensuseleap": + case "centos7": return nil, errors.NotSupportedf("installing mongo on series %q", series) default: if dep.snapChannel == "" { diff --git a/packaging/manager.go b/packaging/manager.go index 3b045f1ec62..892df9e4056 100644 --- a/packaging/manager.go +++ b/packaging/manager.go @@ -20,10 +20,9 @@ type PackageManagerName string // The list of supported package managers. const ( - AptPackageManager PackageManagerName = "apt" - YumPackageManager PackageManagerName = "yum" - ZypperPackageManager PackageManagerName = "zypper" - SnapPackageManager PackageManagerName = "snap" + AptPackageManager PackageManagerName = "apt" + YumPackageManager PackageManagerName = "yum" + SnapPackageManager PackageManagerName = "snap" ) // Dependency is implemented by objects that can provide a series-specific @@ -117,8 +116,6 @@ func newPackageManager(name PackageManagerName) (manager.PackageManager, error) return manager.NewAptPackageManager(), nil case YumPackageManager: return manager.NewYumPackageManager(), nil - case ZypperPackageManager: - return manager.NewZypperPackageManager(), nil case SnapPackageManager: return manager.NewSnapPackageManager(), nil default: @@ -132,8 +129,6 @@ func newPackageConfigurer(name PackageManagerName, series string) (config.Packag return config.NewAptPackagingConfigurer(series), nil case YumPackageManager: return config.NewYumPackagingConfigurer(series), nil - case ZypperPackageManager: - return config.NewZypperPackagingConfigurer(series), nil case SnapPackageManager: return nil, nil default: diff --git a/packaging/manager_test.go b/packaging/manager_test.go index b89583eca46..a33b88466f1 100644 --- a/packaging/manager_test.go +++ b/packaging/manager_test.go @@ -22,27 +22,13 @@ func (s *DependencyManagerTestSuite) SetUpTest(c *gc.C) { } func (s *DependencyManagerTestSuite) TestInstallWithCentos(c *gc.C) { - for _, series := range []string{"centos7", "centos8"} { - s.assertInstallCallsCorrectBinary(c, assertParams{ - series: series, - pkg: "foo", - pm: packaging.YumPackageManager, - expPkgBinary: "yum", - expArgs: []string{ - "--assumeyes", "--debuglevel=1", "install", "foo", - }, - }) - } -} - -func (s *DependencyManagerTestSuite) TestInstallWithOpenSuse(c *gc.C) { s.assertInstallCallsCorrectBinary(c, assertParams{ - series: "opensuseleap", + series: "centos7", pkg: "foo", - pm: packaging.ZypperPackageManager, - expPkgBinary: "zypper", + pm: packaging.YumPackageManager, + expPkgBinary: "yum", expArgs: []string{ - "--quiet", "--non-interactive", "install", "foo", + "--assumeyes", "--debuglevel=1", "install", "foo", }, }) } diff --git a/provider/common/disk.go b/provider/common/disk.go index 286e5e66f6a..8cab4faa349 100644 --- a/provider/common/disk.go +++ b/provider/common/disk.go @@ -11,15 +11,8 @@ import ( // instance, in Gigabytes. This value accommodates the anticipated // size of the initial image, any updates, and future application // data. -func MinRootDiskSizeGiB(os jujuos.OSType) uint64 { - switch os { - case jujuos.Ubuntu, jujuos.CentOS, jujuos.OpenSUSE: - return 8 - // By default we just return a "sane" default, since the error will just - // be returned by the api and seen in juju status - default: - return 8 - } +func MinRootDiskSizeGiB(_ jujuos.OSType) uint64 { + return 8 } // MiBToGiB converts the provided megabytes (base-2) into the nearest diff --git a/provider/lxd/userdata.go b/provider/lxd/userdata.go index 7df75a05407..d11cc4c8e87 100644 --- a/provider/lxd/userdata.go +++ b/provider/lxd/userdata.go @@ -16,7 +16,7 @@ type lxdRenderer struct{} // EncodeUserdata implements renderers.ProviderRenderer. func (lxdRenderer) Render(cfg cloudinit.CloudConfig, os jujuos.OSType) ([]byte, error) { switch os { - case jujuos.Ubuntu, jujuos.CentOS, jujuos.OpenSUSE: + case jujuos.Ubuntu, jujuos.CentOS: bytes, err := renderers.RenderYAML(cfg) return bytes, errors.Trace(err) default: diff --git a/tests/suites/deploy/os.sh b/tests/suites/deploy/os.sh index 86c1a0d5fe2..d7ce13f6b08 100644 --- a/tests/suites/deploy/os.sh +++ b/tests/suites/deploy/os.sh @@ -16,7 +16,7 @@ test_deploy_os() { # https://wiki.centos.org/Cloud/AWS # run "run_deploy_centos7" - run "run_deploy_centos8" + run "run_deploy_centos9" ;; *) echo "==> TEST SKIPPED: deploy_centos - tests for AWS only" @@ -42,17 +42,17 @@ run_deploy_centos7() { # juju add-model test-deploy-centos-west2 aws/us-west-2 - juju metadata add-image --series centos7 ami-0bc06212a56393ee1 + juju metadata add-image --base centos@7 ami-0bc06212a56393ee1 # # There is a specific list of instance types which can be used with # this image. Sometimes juju chooses the wrong one e.g. t3a.medium. # Ensure we use one that is allowed. # - juju deploy ./tests/suites/deploy/charms/centos-dummy-sink --series centos7 --constraints instance-type=t3.medium + juju deploy ./tests/suites/deploy/charms/centos-dummy-sink --base centos@7 --constraints instance-type=t3.medium - base=$(juju status --format=json | jq '.applications."dummy-sink".base') - echo "$base" | check "centos@7" + juju status --format=json | jq '.applications."dummy-sink".base.name' | check "centos" + juju status --format=json | jq '.applications."dummy-sink".base.channel' | check "7" wait_for "dummy-sink" "$(idle_condition "dummy-sink")" @@ -60,13 +60,13 @@ run_deploy_centos7() { destroy_model "test-deploy-centos-west2" } -run_deploy_centos8() { +run_deploy_centos9() { echo echo "==> Checking for dependencies" check_juju_dependencies metadata - name="test-deploy-centos8" + name="test-deploy-centos9" file="${TEST_DIR}/${name}.log" ensure "${name}" "${file}" @@ -75,16 +75,16 @@ run_deploy_centos8() { # Images have been setup and and subscribed for juju-qa aws # in us-east-1. Take care editing the details. # - juju metadata add-image --series centos8 ami-0d6e9a57f6259ba3a + juju metadata add-image --base centos@9 ami-0df2a11dd1fe1f8e3 # # The disk size must be >= 10G to cover the image above. # Ensure we use an instance with enough disk space. # - juju deploy ./tests/suites/deploy/charms/centos-dummy-sink --series centos8 --constraints root-disk=10G + juju deploy ./tests/suites/deploy/charms/centos-dummy-sink --base centos@9 --constraints root-disk=10G - base=$(juju status --format=json | jq '.applications."dummy-sink".base') - echo "$base" | check "centos@8" + juju status --format=json | jq '.applications."dummy-sink".base.name' | check "centos" + juju status --format=json | jq '.applications."dummy-sink".base.channel' | check "9" wait_for "dummy-sink" "$(idle_condition "dummy-sink")" diff --git a/version/current_test.go b/version/current_test.go index 0452194c4c7..e6e6817b487 100644 --- a/version/current_test.go +++ b/version/current_test.go @@ -5,7 +5,6 @@ package version import ( "os/exec" - "runtime" osseries "github.com/juju/os/v2/series" gc "gopkg.in/check.v1" @@ -28,27 +27,13 @@ func (*CurrentSuite) TestCurrentSeries(c *gc.C) { if err != nil { // If the command fails (for instance if we're running on some other // platform) then CurrentSeries should be unknown. - switch runtime.GOOS { - case "darwin": - c.Check(s, gc.Matches, `mavericks|mountainlion|lion|snowleopard`) - default: - currentOS, err := corebase.GetOSFromSeries(s) - c.Assert(err, gc.IsNil) - if s != "n/a" { - // There is no lsb_release command on CentOS. - if currentOS == os.CentOS { - c.Check(s, gc.Matches, `centos\d+`) - } - } - } - } else { - //OpenSUSE lsb-release returns n/a currentOS, err := corebase.GetOSFromSeries(s) c.Assert(err, gc.IsNil) - if string(out) == "n/a" && currentOS == os.OpenSUSE { - c.Check(s, gc.Matches, "opensuseleap") - } else { - c.Assert(string(out), gc.Equals, "Codename:\t"+s+"\n") + // There is no lsb_release command on CentOS. + if s != "n/a" && currentOS == os.CentOS { + c.Check(s, gc.Matches, `centos\d+`) } + } else { + c.Assert(string(out), gc.Equals, "Codename:\t"+s+"\n") } } diff --git a/worker/uniter/runner/context/env.go b/worker/uniter/runner/context/env.go index a6fe2bb7690..91974e69a8d 100644 --- a/worker/uniter/runner/context/env.go +++ b/worker/uniter/runner/context/env.go @@ -111,8 +111,6 @@ func OSDependentEnvVars(paths Paths, env Environmenter) []string { return ubuntuEnv(paths, env) case jujuos.CentOS: return centosEnv(paths, env) - case jujuos.OpenSUSE: - return opensuseEnv(paths, env) case jujuos.GenericLinux: return genericLinuxEnv(paths, env) } @@ -157,27 +155,6 @@ func centosEnv(paths Paths, envVars Environmenter) []string { return env } -func opensuseEnv(paths Paths, envVars Environmenter) []string { - path := appendPath(paths, envVars) - - env := []string{ - "LANG=C.UTF-8", - } - - env = append(env, path...) - - // OpenSUSE 42 does not include patch 20150502 for ncurses 5.9 with - // with terminal definitions for "tmux" and "tmux-256color" - hostSeries, err := series.HostSeries() - if err == nil && hostSeries == "opensuseleap" { - env = append(env, "TERM=screen-256color") - } else { - env = append(env, "TERM=tmux-256color") - } - - return env -} - func genericLinuxEnv(paths Paths, envVars Environmenter) []string { path := appendPath(paths, envVars) diff --git a/worker/uniter/runner/context/env_test.go b/worker/uniter/runner/context/env_test.go index b6a01154a01..1fcb4dc866e 100644 --- a/worker/uniter/runner/context/env_test.go +++ b/worker/uniter/runner/context/env_test.go @@ -296,67 +296,6 @@ func (s *EnvSuite) TestEnvCentos(c *gc.C) { } } -func (s *EnvSuite) TestEnvOpenSUSE(c *gc.C) { - ctrl := gomock.NewController(c) - defer ctrl.Finish() - - state := mocks.NewMockState(ctrl) - state.EXPECT().StorageAttachment(names.NewStorageTag("data/0"), names.NewUnitTag("this-unit/123")).Return(params.StorageAttachment{ - Kind: params.StorageKindBlock, - Location: "/dev/sdb", - }, nil).AnyTimes() - unit := mocks.NewMockHookUnit(ctrl) - unit.EXPECT().Tag().Return(names.NewUnitTag("this-unit/123")).AnyTimes() - - s.PatchValue(&jujuos.HostOS, func() jujuos.OSType { return jujuos.OpenSUSE }) - s.PatchValue(&jujuversion.Current, version.MustParse("1.2.3")) - - // TERM is different for opensuseleap. - for _, testSeries := range []string{"opensuseleap", "opensuse"} { - s.PatchValue(&osseries.HostSeries, func() (string, error) { return testSeries, nil }) - openSUSEVars := []string{ - "LANG=C.UTF-8", - "PATH=path-to-tools:foo:bar", - } - - if testSeries == "opensuseleap" { - openSUSEVars = append(openSUSEVars, "TERM=screen-256color") - } else { - openSUSEVars = append(openSUSEVars, "TERM=tmux-256color") - } - - environmenter := context.NewRemoteEnvironmenter( - func() []string { return []string{} }, - func(k string) string { - switch k { - case "PATH": - return "foo:bar" - } - return "" - }, - func(k string) (string, bool) { - switch k { - case "PATH": - return "foo:bar", true - } - return "", false - }, - ) - - ctx, contextVars := s.getContext(false, state, unit) - paths, pathsVars := s.getPaths() - actualVars, err := ctx.HookVars(paths, false, environmenter) - c.Assert(err, jc.ErrorIsNil) - s.assertVars(c, actualVars, contextVars, pathsVars, openSUSEVars) - - relationVars := s.setRelation(ctx) - secretVars := s.setSecret(ctx) - actualVars, err = ctx.HookVars(paths, false, environmenter) - c.Assert(err, jc.ErrorIsNil) - s.assertVars(c, actualVars, contextVars, pathsVars, openSUSEVars, relationVars, secretVars) - } -} - func (s *EnvSuite) TestEnvGenericLinux(c *gc.C) { ctrl := gomock.NewController(c) defer ctrl.Finish() From 04367afd2ba61d896809ef916150c6b20edd1220 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Mon, 14 Aug 2023 18:56:28 +1000 Subject: [PATCH 10/10] Simplify removeSecrets in apiserver/common/secrets/; --- apiserver/common/secrets/secrets.go | 77 +++++++----- apiserver/common/secrets/secrets_test.go | 32 ++--- .../facades/agent/secretsmanager/secrets.go | 11 +- apiserver/facades/client/secrets/secrets.go | 43 +++---- .../facades/client/secrets/secrets_test.go | 112 +++++++++++++++++- 5 files changed, 194 insertions(+), 81 deletions(-) diff --git a/apiserver/common/secrets/secrets.go b/apiserver/common/secrets/secrets.go index 25b23ccf39e..a02e8c3c74b 100644 --- a/apiserver/common/secrets/secrets.go +++ b/apiserver/common/secrets/secrets.go @@ -13,11 +13,9 @@ import ( "github.com/juju/names/v4" apiservererrors "github.com/juju/juju/apiserver/errors" - "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/cloud" "github.com/juju/juju/core/leadership" corelogger "github.com/juju/juju/core/logger" - "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" "github.com/juju/juju/environs/cloudspec" @@ -539,33 +537,57 @@ func GetSecretMetadata( return result, nil } -// RemoveSecrets removes the specified secrets. -// If removeFromBackend is false, the secrets are only removed from the state and +// RemoveSecretsForAgent removes the specified secrets for agent. +// The secrets are only removed from the state and // the caller must have permission to manage the secret(secret owners remove secrets from the backend on uniter side). -// If removeFromBackend is true, the secrets are removed from the state and +func RemoveSecretsForAgent( + removeState SecretsRemoveState, adminConfigGetter BackendAdminConfigGetter, + authTag names.Tag, args params.DeleteSecretArgs, + canDelete func(*coresecrets.URI) error, +) (params.ErrorResults, error) { + return removeSecrets( + removeState, adminConfigGetter, authTag, args, canDelete, + func(provider.SecretBackendProvider, provider.ModelBackendConfig, []string) error { return nil }, + ) +} + +// RemoveSecretsForClient removes the specified secrets for client. +// The secrets are removed from the state and // backend and the caller must have model admin access. -func RemoveSecrets( - secretsState SecretsMetaState, - removeState SecretsRemoveState, - secretsConsumer SecretsConsumer, - adminConfigGetter BackendAdminConfigGetter, - modelTag names.ModelTag, authorizer facade.Authorizer, authTag names.Tag, leadershipChecker leadership.Checker, - args params.DeleteSecretArgs, removeFromBackend bool, +func RemoveSecretsForClient( + removeState SecretsRemoveState, adminConfigGetter BackendAdminConfigGetter, + authTag names.Tag, args params.DeleteSecretArgs, + canDelete func(*coresecrets.URI) error, +) (params.ErrorResults, error) { + return removeSecrets( + removeState, adminConfigGetter, authTag, args, canDelete, + func(p provider.SecretBackendProvider, cfg provider.ModelBackendConfig, revisions []string) error { + backend, err := p.NewBackend(&cfg) + if err != nil { + return errors.Trace(err) + } + for _, revId := range revisions { + if err = backend.DeleteContent(context.TODO(), revId); err != nil { + return errors.Trace(err) + } + } + return nil + }, + ) +} + +func removeSecrets( + removeState SecretsRemoveState, adminConfigGetter BackendAdminConfigGetter, + authTag names.Tag, args params.DeleteSecretArgs, + canDelete func(*coresecrets.URI) error, + removeSecretsFromBackend func(provider.SecretBackendProvider, provider.ModelBackendConfig, []string) error, ) (params.ErrorResults, error) { removeSecret := func(uri *coresecrets.URI, revisions ...int) ([]coresecrets.ValueRef, error) { if _, err := removeState.GetSecret(uri); err != nil { // Check if the uri exists or not. return nil, errors.Trace(err) } - if removeFromBackend { - if err := authorizer.HasPermission(permission.AdminAccess, modelTag); err != nil { - return nil, errors.Trace(err) - } - // Admins can delete any secret in the model. - return removeState.DeleteSecret(uri, revisions...) - } - _, err := CanManage(secretsConsumer, leadershipChecker, authTag, uri) - if err != nil { + if err := canDelete(uri); err != nil { return nil, errors.Trace(err) } return removeState.DeleteSecret(uri, revisions...) @@ -611,7 +633,6 @@ func RemoveSecrets( // TODO: include unitTag in params.DeleteSecretArgs for operator uniters? // This should be resolved once lp:1991213 and lp:1991854 are fixed. backendCfg, ok := cfgInfo.Configs[backendID] - logger.Criticalf("backendID %q, cfgInfo.Configs: %#v", backendID, cfgInfo.Configs) if !ok { return params.ErrorResults{}, errors.NotFoundf("secret backend %q", backendID) } @@ -619,16 +640,8 @@ func RemoveSecrets( if err != nil { return params.ErrorResults{}, errors.Trace(err) } - if removeFromBackend { - backend, err := provider.NewBackend(&backendCfg) - if err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - for _, revId := range r.RevisionIDs() { - if err = backend.DeleteContent(context.TODO(), revId); err != nil { - return params.ErrorResults{}, errors.Trace(err) - } - } + if err := removeSecretsFromBackend(provider, backendCfg, r.RevisionIDs()); err != nil { + return params.ErrorResults{}, errors.Trace(err) } if err := provider.CleanupSecrets(&backendCfg, authTag, r); err != nil { return params.ErrorResults{}, errors.Trace(err) diff --git a/apiserver/common/secrets/secrets_test.go b/apiserver/common/secrets/secrets_test.go index ed9e414e4bc..5ca63acb05b 100644 --- a/apiserver/common/secrets/secrets_test.go +++ b/apiserver/common/secrets/secrets_test.go @@ -17,7 +17,6 @@ import ( "github.com/juju/juju/apiserver/common/secrets/mocks" "github.com/juju/juju/cloud" "github.com/juju/juju/core/leadership" - "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" "github.com/juju/juju/secrets/provider" @@ -730,11 +729,7 @@ func (s *secretsSuite) TestRemoveSecretsForSecretOwners(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri - secretsMetaState := mocks.NewMockSecretsMetaState(ctrl) removeState := mocks.NewMockSecretsRemoveState(ctrl) - secretsConsumer := mocks.NewMockSecretsConsumer(ctrl) - authorizer := mocks.NewMockAuthorizer(ctrl) - leadershipChecker := mocks.NewMockChecker(ctrl) mockprovider := mocks.NewMockSecretBackendProvider(ctrl) s.PatchValue(&secrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return mockprovider, nil }) @@ -744,8 +739,6 @@ func (s *secretsSuite) TestRemoveSecretsForSecretOwners(c *gc.C) { RevisionID: "rev-666", }}, nil) - secretsConsumer.EXPECT().SecretAccess(uri, names.NewUnitTag("mariadb/0")).Return(coresecrets.RoleManage, nil) - cfg := &provider.ModelBackendConfig{ ControllerUUID: coretesting.ControllerTag.Id(), ModelUUID: coretesting.ModelTag.Id(), @@ -777,16 +770,16 @@ func (s *secretsSuite) TestRemoveSecretsForSecretOwners(c *gc.C) { }, nil } - results, err := secrets.RemoveSecrets( - secretsMetaState, removeState, secretsConsumer, adminConfigGetter, coretesting.ModelTag, - authorizer, names.NewUnitTag("mariadb/0"), - leadershipChecker, + results, err := secrets.RemoveSecretsForAgent( + removeState, adminConfigGetter, + names.NewUnitTag("mariadb/0"), params.DeleteSecretArgs{ Args: []params.DeleteSecretArg{{ URI: expectURI.String(), Revisions: []int{666}, }}, - }, false, + }, + func(*coresecrets.URI) error { return nil }, ) c.Assert(err, jc.ErrorIsNil) c.Assert(results, jc.DeepEquals, params.ErrorResults{ @@ -800,16 +793,11 @@ func (s *secretsSuite) TestRemoveSecretsForModelAdmin(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri - secretsMetaState := mocks.NewMockSecretsMetaState(ctrl) removeState := mocks.NewMockSecretsRemoveState(ctrl) - secretsConsumer := mocks.NewMockSecretsConsumer(ctrl) - authorizer := mocks.NewMockAuthorizer(ctrl) - leadershipChecker := mocks.NewMockChecker(ctrl) mockprovider := mocks.NewMockSecretBackendProvider(ctrl) backend := mocks.NewMockSecretsBackend(ctrl) s.PatchValue(&secrets.GetProvider, func(string) (provider.SecretBackendProvider, error) { return mockprovider, nil }) - authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) removeState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) removeState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ BackendID: "backend-id", @@ -849,16 +837,16 @@ func (s *secretsSuite) TestRemoveSecretsForModelAdmin(c *gc.C) { }, nil } - results, err := secrets.RemoveSecrets( - secretsMetaState, removeState, secretsConsumer, adminConfigGetter, coretesting.ModelTag, - authorizer, names.NewUserTag("foo"), - leadershipChecker, + results, err := secrets.RemoveSecretsForClient( + removeState, adminConfigGetter, + names.NewUserTag("foo"), params.DeleteSecretArgs{ Args: []params.DeleteSecretArg{{ URI: expectURI.String(), Revisions: []int{666}, }}, - }, true, + }, + func(*coresecrets.URI) error { return nil }, ) c.Assert(err, jc.ErrorIsNil) c.Assert(results, jc.DeepEquals, params.ErrorResults{ diff --git a/apiserver/facades/agent/secretsmanager/secrets.go b/apiserver/facades/agent/secretsmanager/secrets.go index 07b2fe55ab4..51bf9f1c3ec 100644 --- a/apiserver/facades/agent/secretsmanager/secrets.go +++ b/apiserver/facades/agent/secretsmanager/secrets.go @@ -354,10 +354,13 @@ func (s *SecretsManagerAPI) updateSecret(arg params.UpdateSecretArg) error { // RemoveSecrets removes the specified secrets. func (s *SecretsManagerAPI) RemoveSecrets(args params.DeleteSecretArgs) (params.ErrorResults, error) { - return commonsecrets.RemoveSecrets( - s.secretsState, s.secretsState, s.secretsConsumer, s.adminConfigGetter, - names.NewModelTag(s.modelUUID), s.authorizer, s.authTag, s.leadershipChecker, - args, false, + return commonsecrets.RemoveSecretsForAgent( + s.secretsState, s.adminConfigGetter, + s.authTag, args, + func(uri *coresecrets.URI) error { + _, err := s.canManage(uri) + return errors.Trace(err) + }, ) } diff --git a/apiserver/facades/client/secrets/secrets.go b/apiserver/facades/client/secrets/secrets.go index 5c018277450..1d9e44ca464 100644 --- a/apiserver/facades/client/secrets/secrets.go +++ b/apiserver/facades/client/secrets/secrets.go @@ -14,7 +14,6 @@ import ( commonsecrets "github.com/juju/juju/apiserver/common/secrets" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" - "github.com/juju/juju/core/leadership" "github.com/juju/juju/core/permission" coresecrets "github.com/juju/juju/core/secrets" "github.com/juju/juju/rpc/params" @@ -266,15 +265,10 @@ func (s *SecretsAPI) CreateSecrets(args params.CreateSecretArgs) (params.StringR return result, nil } -type token struct { - disallow bool -} +type successfulToken struct{} // Check implements lease.Token. -func (t token) Check() error { - if t.disallow { - return apiservererrors.ErrPerm - } +func (t successfulToken) Check() error { return nil } @@ -340,7 +334,7 @@ func fromUpsertParams(p params.UpsertSecretArg) state.UpdateSecretParams { } } return state.UpdateSecretParams{ - LeaderToken: token{}, + LeaderToken: successfulToken{}, Description: p.Description, Label: p.Label, Params: p.Params, @@ -416,22 +410,29 @@ func (s *SecretsAPI) updateSecret(backend provider.SecretsBackend, arg params.Up return errors.Trace(err) } -type leadershipChecker struct{} - -// Check implements lease.Token. -func (t leadershipChecker) LeadershipCheck(applicationId, unitId string) leadership.Token { - return &token{disallow: true} -} - // RemoveSecrets isn't on the v1 API. func (s *SecretsAPIV1) RemoveSecrets(_ struct{}) {} // RemoveSecrets remove user secret. func (s *SecretsAPI) RemoveSecrets(args params.DeleteSecretArgs) (params.ErrorResults, error) { - return commonsecrets.RemoveSecrets( - s.secretsState, s.secretsState, s.secretsConsumer, s.backendConfigGetter, - names.NewModelTag(s.modelUUID), s.authorizer, s.authTag, &leadershipChecker{}, - args, true, + return commonsecrets.RemoveSecretsForClient( + s.secretsState, s.backendConfigGetter, + s.authTag, args, + func(uri *coresecrets.URI) error { + // Only admin can delete user secrets. + if err := s.checkCanAdmin(); err != nil { + return errors.Trace(err) + } + md, err := s.secretsState.GetSecret(uri) + if err != nil { + return errors.Trace(err) + } + // Can only delete model owned(user supplied) secrets. + if md.OwnerTag != names.NewModelTag(s.modelUUID).String() { + return apiservererrors.ErrPerm + } + return nil + }, ) } @@ -470,7 +471,7 @@ func (s *SecretsAPI) secretsGrantRevoke(arg params.GrantRevokeUserSecretArg, op one := func(appName string) error { subjectTag := names.NewApplicationTag(appName) if err := op(uri, state.SecretAccessParams{ - LeaderToken: token{}, + LeaderToken: successfulToken{}, Scope: scopeTag, Subject: subjectTag, Role: coresecrets.RoleView, diff --git a/apiserver/facades/client/secrets/secrets_test.go b/apiserver/facades/client/secrets/secrets_test.go index ef6b3599715..77ca45c91ee 100644 --- a/apiserver/facades/client/secrets/secrets_test.go +++ b/apiserver/facades/client/secrets/secrets_test.go @@ -536,8 +536,10 @@ func (s *SecretsSuite) TestRemoveSecrets(c *gc.C) { uri := coresecrets.NewURI() expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.SuperuserAccess, coretesting.ControllerTag).Return( + errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) - s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{URI: uri, OwnerTag: coretesting.ModelTag.String()}, nil).Times(2) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return([]coresecrets.ValueRef{{ BackendID: "backend-id", RevisionID: "rev-666", @@ -602,14 +604,120 @@ func (s *SecretsSuite) TestRemoveSecrets(c *gc.C) { }) } +func (s *SecretsSuite) TestRemoveSecretsFailedNotModelAdmin(c *gc.C) { + defer s.setup(c).Finish() + s.expectAuthClient() + + uri := coresecrets.NewURI() + expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.SuperuserAccess, coretesting.ControllerTag).Return( + errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(apiservererrors.ErrPerm) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{URI: uri, OwnerTag: names.NewModelTag("1cfde5b3-663d-47bf-8799-71b84fa2df3f").String()}, nil).Times(1) + + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + results, err := facade.RemoveSecrets(params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results.Results[0].Error.Message, jc.DeepEquals, "permission denied") +} + +func (s *SecretsSuite) TestRemoveSecretsFailedNotModelOwned(c *gc.C) { + defer s.setup(c).Finish() + s.expectAuthClient() + + uri := coresecrets.NewURI() + expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.SuperuserAccess, coretesting.ControllerTag).Return( + errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) + s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{URI: uri, OwnerTag: names.NewModelTag("1cfde5b3-663d-47bf-8799-71b84fa2df3f").String()}, nil).Times(2) + + facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer, + func() (*provider.ModelBackendConfigInfo, error) { + return &provider.ModelBackendConfigInfo{ + ActiveID: "backend-id", + Configs: map[string]provider.ModelBackendConfig{ + "backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "active-type", + Config: map[string]interface{}{"foo": "active-type"}, + }, + }, + "other-backend-id": { + ControllerUUID: coretesting.ControllerTag.Id(), + ModelUUID: coretesting.ModelTag.Id(), + ModelName: "some-model", + BackendConfig: provider.BackendConfig{ + BackendType: "other-type", + Config: map[string]interface{}{"foo": "other-type"}, + }, + }, + }, + }, nil + }, + func(cfg *provider.ModelBackendConfig) (provider.SecretsBackend, error) { + c.Assert(cfg.Config, jc.DeepEquals, provider.ConfigAttrs{"foo": cfg.BackendType}) + return s.secretsBackend, nil + }, s.authorizer, s.authTag) + c.Assert(err, jc.ErrorIsNil) + results, err := facade.RemoveSecrets(params.DeleteSecretArgs{ + Args: []params.DeleteSecretArg{{ + URI: expectURI.String(), + Revisions: []int{666}, + }}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results.Results[0].Error.Message, jc.DeepEquals, "permission denied") +} + func (s *SecretsSuite) TestRemoveSecretRevision(c *gc.C) { defer s.setup(c).Finish() s.expectAuthClient() uri := coresecrets.NewURI() expectURI := *uri + s.authorizer.EXPECT().HasPermission(permission.SuperuserAccess, coretesting.ControllerTag).Return( + errors.WithType(apiservererrors.ErrPerm, authentication.ErrorEntityMissingPermission)) s.authorizer.EXPECT().HasPermission(permission.AdminAccess, coretesting.ModelTag).Return(nil) - s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{}, nil) + s.secretsState.EXPECT().GetSecret(&expectURI).Return(&coresecrets.SecretMetadata{URI: uri, OwnerTag: coretesting.ModelTag.String()}, nil).Times(2) s.secretsState.EXPECT().DeleteSecret(&expectURI, []int{666}).Return(nil, nil) facade, err := apisecrets.NewTestAPI(s.secretsState, s.secretConsumer,