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

Added changes for deletion of sshKey/sshKeys #192

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

bkhadars
Copy link
Contributor

Added Changes:

  1. To delete all sshkey's with a specific string pattern
  2. To Select multiple key's with specific pattern and delete it

@ppc64le-cloud-bot ppc64le-cloud-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 11, 2021
@bkhadars bkhadars force-pushed the ssh_key branch 2 times, most recently from a1fc2ea to 0b603bb Compare November 11, 2021 03:59
Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

Initial review comments

cmd/delete/key/key.go Outdated Show resolved Hide resolved
pkg/client/key/key.go Outdated Show resolved Hide resolved
pkg/client/key/key.go Outdated Show resolved Hide resolved
pkg/client/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

Lets move to purge command as discussed

Comment on lines 42 to 55
# Delete all the ssh keys which are created before 12hrs
pvsadm delete key --instance-name upstream-core --before 12h

#Delete all the ssh keys created since 30m
pvsadm delete key --instance-name upstream-core --since 30m

#Delete all ssh keys starts with rdr-
pvsadm delete key --instance-name upstream-core --regex "^rdr-.*"

#Delete all ssh keys without asking confirmation
pvsadm delete key --instance-name upstream-core --noprompt

#Delete all ssh keys and ignore if any errors during the delete operation
pvsadm delete key --instance-name upstream-core --ignore-errors
Copy link
Member

Choose a reason for hiding this comment

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

keys api is not tied with any instance, hence wondering if we really need the instance-name or instance-id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From IBM UI, we can check ssh_keys irrespective of the instances, but powervs sdk needs instance id for getting the ssh keys..
c.session.Power.PCloudTenantsSSHKeys.PcloudTenantsSshkeysGetall(params, ibmpisession.NewAuth(c.session, c.instanceID))

PcloudTenantsSshkeysGetall function needs information about instanceid, hence I have added the instance-name

Copy link
Member

Choose a reason for hiding this comment

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

can we send an empty or some dummy value there? because API doesn't I feel.

cmd/delete/key/key.go Outdated Show resolved Hide resolved
pkg/client/key/key.go Outdated Show resolved Hide resolved
cmd/delete/key/key.go Outdated Show resolved Hide resolved
@mkumatag
Copy link
Member

@bkhadars is this ready for review?

@bkhadars
Copy link
Contributor Author

@bkhadars is this ready for review?

yes, @mkumatag

Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@ppc64le-cloud-bot ppc64le-cloud-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2022
@ppc64le-cloud-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bkhadars, mkumatag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ppc64le-cloud-bot ppc64le-cloud-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2022
@ppc64le-cloud-bot ppc64le-cloud-bot merged commit 9d7a980 into ppc64le-cloud:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants