VPC key create and delete operations added #1085

Madhan-SWE · 2023-02-08T10:28:17Z

What this PR does / why we need it:
This is for creating and deleting key from VPC.

Create key

%./capibmadm vpc key create --name test-key --key-path /Users/madhankumar/.ssh/id_rsa.pub --region eu-de 

VPC Key created successfully, key-name="test-key"

% ./capibmadm vpc key create  --name test-key --public-key "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDHeTZS7Pxr/43AG1Z0rVktiisut+XTqODsZMBFwJX3AJUmp4K21GzOYaBYreAIAZht36I7Qw8Kv5QtqMDcZl00dkQZBHR4Gs7fQUpjZh5z6yDSm2v5SjUf7POk3kixEbyUuE1o0Hik0acaEk/18r09FHuelSsB2UvaBDjbF/1EMzrgHJ8LgnH9Md+Y/I6fb4NE3Jryrs20rC5pTxCkzX2K7NYo1oduv9pkGA2tAdukCFPgX0HjXdYP1h54MJxWU/EtMgjz85HjrWH0NYrQEV+uyh1RiBeDKRzBtqqXguXIomZhB2k0WdMPSNM5G/tkXXCM1aO1ZgCzfrn1mP1xp+gzBSjTJi5oX2KPsffP5oyTd09XRWjsjcbAUZEYbcRi8dV3CwtGB8vPFqWtzoE0NUkGTe+e+y81sEPbbikUt9PM+8MQmVDzSEfxYFVem8CJfCRkE3/egfkOP4cRo+px3y4nQngJoKDmuJHjs3BbcgZNxDBCNA44FMgxhaxEUgNkkjM= [email protected]" --region "eu-de"

VPC Key created successfully, key-name="test-key"

Delete key

% ./capibmadm vpc key delete --name test-key --region eu-de

VPC Key deleted succssfully, key-name="test-key"

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

netlify · 2023-02-08T10:28:22Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`2073ae9`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/63f4644f2a5c180008ec57f6
😎 Deploy Preview	https://deploy-preview-1085--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Karthik-K-N · 2023-02-09T05:54:20Z

Could you please refer to Power VS key command PR: #1087 and make the necessary changes to make it similar.

Also docs are missing in the PR.

Prajyot-Parab · 2023-02-09T06:46:15Z

@Madhan-SWE Need to update docs here - Doc ref
For more info, you can refer to Kishen's PR, reach out to me for any queries

cmd/capibmadm/cmd/vpc/key/create.go

cmd/capibmadm/cmd/vpc/key/delete.go

cmd/capibmadm/cmd/vpc/key/create.go

k8s-ci-robot · 2023-02-09T11:17:03Z

@Prajyot-Parab: GitHub didn't allow me to request PR reviews from the following users: dharaneeshvrd.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

capibmadm vpc key create --name <key-name> --region <region> --resource-group-name <resource-group-name>

/cc @Karthik-K-N @dharaneeshvrd

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cmd/capibmadm/cmd/vpc/key/delete.go

k8s-ci-robot · 2023-02-09T11:18:34Z

@Prajyot-Parab: GitHub didn't allow me to request PR reviews from the following users: dharaneeshvrd.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

capibmadm vpc key delete --name <key-name> --region <region>`,
lets take key name as input via flag.

/cc @Karthik-K-N @dharaneeshvrd

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cmd/capibmadm/cmd/vpc/key/delete.go

Prajyot-Parab · 2023-02-09T11:24:56Z

cmd/capibmadm/cmd/vpc/key/delete.go

+
+func deleteKey(keyDeleteOption keyDeleteOptions) error {
+	log := logf.Log
+	v1, err := vpc.NewV1Client(options.GlobalOptions.VPCRegion)


Lets rename v1 to something more descriptive.

v1 is renamed to vpcservice

v1 is rename to vpcClient

cmd/capibmadm/cmd/vpc/key/delete.go

Prajyot-Parab · 2023-02-09T11:28:44Z

cmd/capibmadm/cmd/vpc/key/create.go

+
+func createKey(ctx context.Context, keyCreateOption keyCreateOptions) error {
+	log := logf.Log
+	v1, err := vpc.NewV1Client(options.GlobalOptions.VPCRegion)


lets rename v1 to something more descriptive.

v1 is renamed to vpcservice

v1 is rename to vpcClient

cmd/capibmadm/cmd/vpc/key/create.go

Karthik-K-N · 2023-02-09T12:33:35Z

cmd/capibmadm/cmd/vpc/key/create.go

+# Create key in VPC
+export IBMCLOUD_API_KEY=<api-key>
+capibmadm vpc key create --name <key-name> --region <region> --resource-group-name <resource-group-name>
+ --public-key <public-key-string> `,


should be withing quotations right, like --public-key-string ""

yes.
can we represent like this?

--public-key "<public-key-string>"

any suggestions?

@Karthik-K-N @Prajyot-Parab

Yes we need to represent like

--public-key "<public-key-string>"

Karthik-K-N · 2023-02-09T12:34:44Z

cmd/capibmadm/cmd/vpc/key/create.go

+	options.AddCommonFlags(cmd)
+	var keyCreateOption keyCreateOptions
+	cmd.Flags().StringVar(&keyCreateOption.name, "name", keyCreateOption.name, "Key Name")
+	cmd.Flags().StringVar(&keyCreateOption.resourceGroupName, "resource-group-name", keyCreateOption.resourceGroupName, "IBM cloud resource group name")


resource-group-name flag is already available under vpc: https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/blob/main/cmd/capibmadm/cmd/vpc/vpc.go#L34, so we can remove it from here

Karthik-K-N · 2023-02-09T12:35:16Z

cmd/capibmadm/cmd/vpc/key/create.go

+
+func createKey(ctx context.Context, keyCreateOption keyCreateOptions) error {
+	log := logf.Log
+	vpcservice, err := vpc.NewV1Client(options.GlobalOptions.VPCRegion)


lets name is as vpcService.

@Karthik-K-N how about vpcClient or vpcClientService?

I feel vpcClient would be better.

yeah, we can go ahead with vpcClient,

v1 is renamed to vpcClient

Karthik-K-N · 2023-02-09T12:36:54Z

cmd/capibmadm/cmd/vpc/key/create.go

+
+	key, _, err := vpcservice.CreateKey(options)
+	if err == nil {
+		log.Info("VPC Key created successfully,", "key id", *key.ID)


Better we log key name instead ID,

Karthik-K-N · 2023-02-09T12:38:13Z

cmd/capibmadm/cmd/vpc/key/delete.go

+	}
+
+	if keyID == "" {
+		return fmt.Errorf("key with the specified name could not be found")


Suggested change

return fmt.Errorf("key with the specified name could not be found")

return fmt.Errorf("specified key %s could not be found", keyDeleteOption.name)

Karthik-K-N · 2023-02-09T12:40:00Z

cmd/capibmadm/cmd/vpc/key/delete.go

+
+	_, err = vpcservice.DeleteKey(options)
+	if err == nil {
+		log.Info("VPC Key deleted succssfully, ", " key name", keyDeleteOption.name)


Lets remove empty spaces and lets change "key name" to "key-name"

Prajyot-Parab · 2023-02-10T05:28:58Z

/test all

cmd/capibmadm/cmd/vpc/key/create.go

Prajyot-Parab · 2023-02-14T16:04:41Z

@Madhan-SWE please fix the conflicts.

docs/book/src/topics/capibmadm/vpc/key.md

Co-authored-by: Prajyot Parab <[email protected]>

Prajyot-Parab

/lgtm

k8s-ci-robot · 2023-02-15T14:40:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Madhan-SWE, Prajyot-Parab
Once this PR has been reviewed and has the lgtm label, please assign mkumatag for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

cmd/capibmadm/cmd/vpc/key/doc.go

cmd/capibmadm/cmd/vpc/key/create.go

cmd/capibmadm/cmd/vpc/key/delete.go

Co-authored-by: Manjunath Kumatagi <[email protected]> Co-authored-by: Prajyot Parab <[email protected]>

k8s-ci-robot · 2023-02-20T11:34:30Z

New changes are detected. LGTM label has been removed.

Madhan-SWE · 2023-02-22T09:34:37Z

This PR contains merge commits which can cause rebasing issues. Created a new PR with all the required changes in a single commit : #1113

k8s-ci-robot · 2023-03-01T06:35:47Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Madhan-SWE · 2023-03-02T09:31:17Z

/close

k8s-ci-robot · 2023-03-02T09:31:22Z

@Madhan-SWE: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added the area/provider/ibmcloud Issues or PRs related to ibmcloud provider label Feb 8, 2023

k8s-ci-robot requested review from gyliu513 and jichenjc February 8, 2023 10:28

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 8, 2023

Madhan-SWE force-pushed the vpc-key-create branch 2 times, most recently from 22f635e to ea49055 Compare February 9, 2023 04:29

Prajyot-Parab reviewed Feb 9, 2023

View reviewed changes

cmd/capibmadm/cmd/vpc/key/create.go Show resolved Hide resolved

Prajyot-Parab reviewed Feb 9, 2023

View reviewed changes

cmd/capibmadm/cmd/vpc/key/delete.go Outdated Show resolved Hide resolved

Prajyot-Parab reviewed Feb 9, 2023

View reviewed changes

cmd/capibmadm/cmd/vpc/key/create.go Outdated Show resolved Hide resolved

k8s-ci-robot requested a review from Karthik-K-N February 9, 2023 11:17