Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Add show user command #83

Merged
merged 4 commits into from
Nov 13, 2023
Merged

Add show user command #83

merged 4 commits into from
Nov 13, 2023

Conversation

tjmoore4
Copy link
Contributor

@tjmoore4 tjmoore4 commented Nov 3, 2023

Adds a command to show the contents of the pguser Secrets.

Issue: PGO-470

internal/cmd/show.go Outdated Show resolved Hide resolved
internal/cmd/show.go Outdated Show resolved Hide resolved
internal/cmd/show.go Outdated Show resolved Hide resolved
@tjmoore4 tjmoore4 force-pushed the show-user branch 2 times, most recently from 6a5e8fa to 49ef5e5 Compare November 3, 2023 20:44
internal/cmd/show.go Outdated Show resolved Hide resolved
@tony-landreth
Copy link
Contributor

It doesn't look like the command works when I set the user through the spec:

spec:
  users:
    - name: rhino
      databases:
        - zoo

@benjaminjb
Copy link
Collaborator

It doesn't look like the command works when I set the user through the spec:

spec:
  users:
    - name: rhino
      databases:
        - zoo

What are you seeing?

If I set two users in the spec, one with the databases field filled out, like

    users:
    - name: hippo
      databases:
      - zoo
    - name: rhino

then the show user command just lists them (which is pretty boring now that I think of it):

➜ ./bin/kubectl-pgo show user hippo -n postgres-operator                       
CLUSTER  USERNAME
-------  --------
hippo    rhino
hippo    hippo

And then if I ask for the connection info, I get the databases[0] listed, as per the secret:

➜  ./bin/kubectl-pgo show user hippo hippo -n postgres-operator --show-connection-info
WARNING: This command will show sensitive password information.
Are you sure you want to continue? (yes/no): yes
Connection information for hippo for hippo cluster
Connection info string:
    dbname=zoo host=hippo-primary.postgres-operator.svc port=5432 user=hippo password=<password>
Connection URL:
    postgres://<password>[email protected]:5432/zoo

@benjaminjb
Copy link
Collaborator

The show user without connection info is pretty boring, but I think we have some ideas abt making that command more like v4 in the future, i.e., pull info from the db.

@benjaminjb
Copy link
Collaborator

Current usage: Screen Shot 2023-11-09 at 2 44 54 PM

@@ -87,7 +92,7 @@ HA

// Print the pgbackrest info output received.
cmd.Printf("BACKUP\n\n")
if stdout, stderr, err := showBackup(config, args, "text", ""); err != nil {
if stdout, stderr, err := getBackup(config, args, "text", ""); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ Not a blocker, but I'd like function renames unrelated to the current feature to be in a separate commit (same for getHA as well).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed new commits to separate that.

}

// Set up a tabwriter that writes to stdout, with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ extra newline

" user=" + string(secret.Data["user"]) +
" password=" + string(secret.Data["password"]))
cmd.Println("Connection URL:")
cmd.Println(" postgres://" + string(secret.Data["password"]) +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Should we have a user value here? For a cluster named hippo (and based on this example ), I'd expect something like

Connection URL:
    postgres://user:[email protected]:5432/hippo

So the code might be

cmd.Println("    postgres://" + string(secret.Data["user"]) + ":" + string(secret.Data["password"]) +

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, good catch

Adds a command to show the contents of the pguser Secrets.

Issue: PGO-470
- Replace 'containsString' function with slices.Contains.
- Move 'confirm' function to util package.
@benjaminjb benjaminjb force-pushed the show-user branch 2 times, most recently from 7e930f7 to d57bd86 Compare November 13, 2023 19:11
Copy link
Contributor Author

@tjmoore4 tjmoore4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Updates look good to me.

@tjmoore4 tjmoore4 merged commit 34df536 into CrunchyData:main Nov 13, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants