-
Notifications
You must be signed in to change notification settings - Fork 87
CLOUDP-57841: Delete dbusers C/OM #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
package cli | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
|
||
"github.com/AlecAivazis/survey/v2" | ||
|
@@ -24,7 +25,7 @@ import ( | |
|
||
const ( | ||
fallbackSuccessMessage = "'%s' deleted\n" | ||
fallbackFailMessage = "'%s' not deleted\n" | ||
fallbackFailMessage = "entry not deleted" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. other fail messages don't interpolate so making the fall back compatible |
||
) | ||
|
||
type globalOpts struct { | ||
|
@@ -58,7 +59,7 @@ func (opts *globalOpts) OrgID() string { | |
} | ||
|
||
// deleteOpts options required when deleting a resource. | ||
// A command can embed this structure and then safely rely on the methods Confirm, DeleteFromProject or Delete | ||
// A command can compose this struct and then safely rely on the methods Confirm, or Delete | ||
// to manage the interactions with the user | ||
type deleteOpts struct { | ||
entry string | ||
|
@@ -67,83 +68,31 @@ type deleteOpts struct { | |
failMessage string | ||
} | ||
|
||
// DeleterFromProject a function to delete from the store. | ||
type DeleterFromProject func(projectID string, entry string) error | ||
|
||
// DeleterFromProjectAuthDB a function to delete from the store. | ||
type DeleterFromProjectAuthDB func(authDB string, projectID string, entry string) error | ||
|
||
// DeleteFromProject deletes a resource from a project, it expects a callback | ||
// Delete deletes a resource not associated to a project, it expects a callback | ||
// that should perform the deletion from the store. | ||
func (opts *deleteOpts) DeleteFromProject(d DeleterFromProject, projectID string) error { | ||
func (opts *deleteOpts) Delete(d interface{}, a ...string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function can do now all three functions |
||
if !opts.confirm { | ||
opts.printFailMessage() | ||
fmt.Println(opts.FailMessage()) | ||
return nil | ||
} | ||
err := d(projectID, opts.entry) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
opts.printSuccessMessage() | ||
|
||
return nil | ||
} | ||
|
||
// DeleterFromProjectAuthDB deletes a resource from a project, it expects a callback | ||
// that should perform the deletion from the store. | ||
func (opts *deleteOpts) DeleterFromProjectAuthDB(d DeleterFromProjectAuthDB, authDB, projectID string) error { | ||
if !opts.confirm { | ||
opts.printFailMessage() | ||
return nil | ||
var err error | ||
switch f := d.(type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wow! 👏 😮 |
||
case func(string) error: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for some reason I could not use types here so getting rid of them |
||
err = f(opts.entry) | ||
case func(string, string) error: | ||
err = f(a[0], opts.entry) | ||
case func(string, string, string) error: | ||
err = f(a[0], a[1], opts.entry) | ||
default: | ||
return errors.New("invalid") | ||
} | ||
err := d(authDB, projectID, opts.entry) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
opts.printSuccessMessage() | ||
|
||
return nil | ||
} | ||
|
||
// printSuccessMessage prints a success message | ||
func (opts *deleteOpts) printSuccessMessage() { | ||
if opts.successMessage != "" { | ||
fmt.Printf(opts.successMessage, opts.entry) | ||
} else { | ||
fmt.Printf(fallbackSuccessMessage, opts.entry) | ||
} | ||
} | ||
|
||
// printFailMessage prints a fail message | ||
func (opts *deleteOpts) printFailMessage() { | ||
if opts.successMessage != "" { | ||
fmt.Printf(opts.failMessage, opts.entry) | ||
} else { | ||
fmt.Printf(fallbackFailMessage, opts.entry) | ||
} | ||
} | ||
|
||
// Deleter a function to delete from the store. | ||
type Deleter func(entry string) error | ||
|
||
// Delete deletes a resource not associated to a project, it expects a callback | ||
//// that should perform the deletion from the store. | ||
func (opts *deleteOpts) Delete(d Deleter) error { | ||
if !opts.confirm { | ||
opts.printFailMessage() | ||
return nil | ||
} | ||
err := d(opts.entry) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
opts.printSuccessMessage() | ||
fmt.Printf(opts.SuccessMessage(), opts.entry) | ||
|
||
return nil | ||
} | ||
|
@@ -157,3 +106,19 @@ func (opts *deleteOpts) Confirm() error { | |
prompt := prompts.NewDeleteConfirm(opts.entry) | ||
return survey.AskOne(prompt, &opts.confirm) | ||
} | ||
|
||
// SuccessMessage gets the set success message or the default value | ||
func (opts *deleteOpts) SuccessMessage() string { | ||
if opts.successMessage != "" { | ||
return opts.successMessage | ||
} | ||
return fallbackSuccessMessage | ||
} | ||
|
||
// FailMessage gets the set fail message or the default value | ||
func (opts *deleteOpts) FailMessage() string { | ||
if opts.failMessage != "" { | ||
return opts.failMessage | ||
} | ||
return fallbackFailMessage | ||
} | ||
Comment on lines
+111
to
+124
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. making this just getters and leaving the printing out |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
// Copyright 2020 MongoDB Inc | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package cli | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/mongodb/mongocli/internal/config" | ||
"github.com/mongodb/mongocli/internal/convert" | ||
"github.com/mongodb/mongocli/internal/flags" | ||
"github.com/mongodb/mongocli/internal/messages" | ||
"github.com/mongodb/mongocli/internal/store" | ||
"github.com/mongodb/mongocli/internal/usage" | ||
"github.com/spf13/cobra" | ||
) | ||
|
||
type opsManagerDBUsersDeleteOpts struct { | ||
*globalOpts | ||
*deleteOpts | ||
authDB string | ||
store store.AutomationPatcher | ||
} | ||
|
||
func (opts *opsManagerDBUsersDeleteOpts) init() error { | ||
if opts.ProjectID() == "" { | ||
return errMissingProjectID | ||
} | ||
|
||
var err error | ||
opts.store, err = store.New() | ||
return err | ||
} | ||
|
||
func (opts *opsManagerDBUsersDeleteOpts) Run() error { | ||
current, err := opts.store.GetAutomationConfig(opts.ProjectID()) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
convert.RemoveUser(current, opts.entry, opts.authDB) | ||
|
||
if err = opts.store.UpdateAutomationConfig(opts.ProjectID(), current); err != nil { | ||
return err | ||
} | ||
|
||
fmt.Print(messages.DeploymentStatus(config.OpsManagerURL(), opts.ProjectID())) | ||
|
||
return nil | ||
} | ||
|
||
// mongocli atlas dbuser(s) delete <username> [--projectId projectId] | ||
func OpsManagerDBUsersDeleteBuilder() *cobra.Command { | ||
opts := &opsManagerDBUsersDeleteOpts{ | ||
globalOpts: newGlobalOpts(), | ||
deleteOpts: &deleteOpts{ | ||
successMessage: "DB user '%s' deleted\n", | ||
failMessage: "DB user not deleted", | ||
}, | ||
} | ||
cmd := &cobra.Command{ | ||
Use: "delete [username]", | ||
Short: "Delete a database user for a project.", | ||
Aliases: []string{"rm"}, | ||
Args: cobra.ExactArgs(1), | ||
PreRunE: func(cmd *cobra.Command, args []string) error { | ||
if err := opts.init(); err != nil { | ||
return err | ||
} | ||
opts.entry = args[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [qa] Do you think it is better to set the arg inside PreRun or Run?. I am quite sure that we are setting that arg inside Run in other commands. We should decide one place and change all the other commands in accordance with that decision. I honestly think that the preRun would be a better place 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we definitely need to agree on this, I've moved it around based on need, the prompt makes sense to run on the pre run so we need the entry set before prompting, but some validations don't run before preRun, I know flags don't do it but I can't remember if the number of args validation does, if we can confirm it does then we should move them all to a preRun |
||
return opts.Confirm() | ||
}, | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
return opts.Run() | ||
}, | ||
} | ||
|
||
cmd.Flags().StringVar(&opts.authDB, flags.AuthDB, convert.AdminDB, usage.AuthDB) | ||
|
||
cmd.Flags().StringVar(&opts.projectID, flags.ProjectID, "", usage.ProjectID) | ||
|
||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
// Copyright 2020 MongoDB Inc | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package cli | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/golang/mock/gomock" | ||
"github.com/mongodb/mongocli/internal/fixtures" | ||
"github.com/mongodb/mongocli/internal/mocks" | ||
) | ||
|
||
func TestOpsManagerDBUserDelete_Run(t *testing.T) { | ||
ctrl := gomock.NewController(t) | ||
mockStore := mocks.NewMockAutomationPatcher(ctrl) | ||
|
||
defer ctrl.Finish() | ||
|
||
expected := fixtures.AutomationConfig() | ||
|
||
createOpts := &opsManagerDBUsersDeleteOpts{ | ||
globalOpts: newGlobalOpts(), | ||
deleteOpts: &deleteOpts{ | ||
confirm: true, | ||
entry: "test", | ||
successMessage: "DB user '%s' deleted\n", | ||
}, | ||
authDB: "admin", | ||
store: mockStore, | ||
} | ||
|
||
mockStore. | ||
EXPECT(). | ||
GetAutomationConfig(createOpts.projectID). | ||
Return(expected, nil). | ||
Times(1) | ||
|
||
mockStore. | ||
EXPECT(). | ||
UpdateAutomationConfig(createOpts.projectID, expected). | ||
Return(nil). | ||
Times(1) | ||
|
||
err := createOpts.Run() | ||
if err != nil { | ||
t.Fatalf("Run() unexpected error: %v", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we missed this in the last PR