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.

Sign up for GitHub

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

Jump to bottom

cli: iam destroy #946

Merged

miampf merged 69 commits into main from feat/iam-destroy-user

Feb 24, 2023

Contributor

miampf commented Jan 11, 2023 •

edited

Loading

Proposed change(s)

Add the new iam destroy subcommand to destroy IAM configuration with the Constellation CLI
Rename the DestroyCluster function in terraform.go to Destroy

This change will allow users to delete their IAM configurations using the constellation CLI instead of the terraform CLI.

Checklist

Update docs
Add labels (e.g., for changelog category)
Link to Milestone

CLAassistant commented Jan 11, 2023 •

edited

Loading

All committers have signed the CLA.

netlify bot commented Jan 11, 2023 •

edited

Loading

✅ Deploy Preview for constellation-docs canceled.

Name	Link
🔨 Latest commit	`82c0eaa`
🔍 Latest deploy log	https://app.netlify.com/sites/constellation-docs/deploys/63f888be169bb50008e1b25f

miampf added the feature label

miampf added this to the v.2.5.0 milestone

3u13r modified the milestones: v.2.5.0, v2.6.0

miampf force-pushed the feat/iam-destroy-user branch from 0d24580 to c43ebe9 Compare

January 27, 2023 15:19

miampf marked this pull request as ready for review

January 27, 2023 17:10

miampf requested review from thomasten, daniel-weisse and katexochen as code owners

January 27, 2023 17:10

daniel-weisse reviewed

View reviewed changes

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/terraform/terraform.go Outdated Show resolved Hide resolved

thomasten reviewed

View reviewed changes

cli/internal/cmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamcreateaws.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamcreateazure.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamcreategcp.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

docs/docs/workflows/config.md Outdated Show resolved Hide resolved

katexochen requested changes

View reviewed changes

Member

katexochen left a comment •

edited by miampf

Loading

Some things left to do:

Check if there is a running cluster. We should ensure that we call constellation terminate before we allow a IAM destroy. You can use the files that are deleted in cmd/terminate.go as indicator.
Only ask user once for confirmation. We should check for the gcpServiceAccountKey before asking the user for confirmation, and state that the file will be deleted there.
We should add some debug output like the other commands have.
Deletion of the gcpKey file can be done directly in the cmd package, no need to have a method for that in cloudcmd. Maybe make the cloudcmd func return the gcpKey directly

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy_test.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy_test.go Outdated Show resolved Hide resolved

cli/internal/cmd/cloud_test.go Show resolved Hide resolved

thomasten closed this

thomasten reopened this

katexochen self-assigned this

daniel-weisse reviewed

View reviewed changes

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

daniel-weisse approved these changes

View reviewed changes

Member

daniel-weisse left a comment

Looks good to me

Just a heads up: cli/internal/cmd/iam.go has undergone some changes (mainly the removal of createiam<CSP>.go files so there will be merge conflicts

katexochen mentioned this pull request

cli: fix iam rollback #1148

Merged

2 tasks

miampf requested a review from katexochen

February 10, 2023 14:20

katexochen requested changes

View reviewed changes

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cloudcmd/iam.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy_test.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy_test.go Outdated Show resolved Hide resolved

cli/internal/cmd/iamdestroy_test.go Show resolved Hide resolved

Member

katexochen commented Feb 10, 2023

Please rebase before working on the changes requested. 🐳

miampf force-pushed the feat/iam-destroy-user branch from 564b1e8 to 6798ff2 Compare

February 16, 2023 14:30

katexochen reviewed

View reviewed changes

cli/internal/cloudcmd/iam_test.go Outdated Show resolved Hide resolved

miampf requested a review from katexochen

February 22, 2023 14:12

katexochen approved these changes

View reviewed changes

Member

katexochen left a comment

lgtm! 🐳

miampf added 3 commits

February 24, 2023 10:20


          added iam destroy subcommand

b13ae6d


          moved DestroyIAMUser into cloudcmd


          added subcommand to parent

1c8aee8

miampf added 25 commits

February 24, 2023 10:28


          moved delete to cmd and added debug output

2c9d89a


          looking out for running cluster by checking filenames

1621c4b


          renamed and fixed unit test

3638ff2


          "IAM user" -> "IAM configuration"

e9930c5


          simplified NewIAMDestroyer and changed unit testing

1d76c1b


          renamed RunGetTfstateSaKey to GetTfstateServiceAccountKey

ecdb0a0


          merged functions and fixed bad previous renaming

7d87c3c


          removed newline

a38f80a


          added test case for an error in CleanUpWorkspace

376dd01


          fixed wording of unit test

fa39d05


          added checks if functions were called

8c97d4b


          removed unnecessary debug prints

5033ace


          return error instead of printing and returning nil

4cd4c3c


          removed newline

810680d


          changed error string return from errors.New to fmt.Errorf

b1a676d


          formatted prints of filenames with %q

1078dfa


          removed unnecessary second check for gcp key file

5ab9c21


          abort if you cannot delete gcp key file

c101192


          added debug message if keys are not the same

68ed2ef


          moved file handler creation into their own functions

6f59dc8


          moved other file handler creation into a function as well

c138250


          Don't abort if keys don't match

ee0a918


          removed unnecessary type casts from unit test

e5f13b7


          cleaned up unit tests a bit more

bc38770


          changed missed unit test that github hid from me

fe2c696

miampf force-pushed the feat/iam-destroy-user branch from bfae283 to fe2c696 Compare

February 24, 2023 09:39


          renamed function

82c0eaa

miampf merged commit 5137e9f into main

miampf deleted the feat/iam-destroy-user branch

February 24, 2023 10:36

miampf added a commit that referenced this pull request


          cli: iam destroy (#946)

b7cfded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature