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

Deploy & other basic commands #4

Merged
merged 25 commits into from
Oct 16, 2019
Merged

Deploy & other basic commands #4

merged 25 commits into from
Oct 16, 2019

Conversation

joshmarsh
Copy link
Contributor

@joshmarsh joshmarsh commented Oct 15, 2019

Goal

POC the core functionality of the CLI.

Important Notes

  • I intend to clean up the code more as I start writing tests. We've gone back and forth a bit on CLI requirements recently, and there was a lot of discovery work involved in this. Therefore I neglected proper error handling, logging, and testing in favor of putting more functionality into the POC so as to drive alignment.
  • The markdown files are autogenerated. I suggest using the file filter to filter them out.
  • I'm continuing to look into patterns for inversion of control and dependency injection in go. The current solution was a bit rushed. Suggestions are welcome.

Current Functionality:

  1. dce init uses an interactive prompt to generate a config file
  2. dce deploy deploys dce to a new master account based on credentials in the config file. Terraform and code artifacts are downloaded from the latest github release.
  3. dce auth opens a web browser to the authentication (SSO) url provided in the config file. User is instructed to login and then copy/paste their API credentials into the terminal. This is not yet integrated with the backend auth solution.
  4. dce accounts add --account-id <string> --admin-role-arn <string> adds an existing account to the accounts pool Bug: The codebuild reset is currently failing to trigger on master accounts deployed by the cli.
  5. dce leases create --budget-amount <float> --budget-currency <string> --email <stringArray> --principle-id <string> creates a lease
  6. dce leases login [--account-id <string> | --lease-id <string>] --open-browser returns temporary credentials for a leased account. Opens a browser to the console if the --open-browser flag is present. This is not yet integrated with the backend auth solution.
  7. dce leases end --account-id <string> --principle-id <string> ends a lease
  8. dce accounts remove --account-id <string>

Next Steps:

  1. Fix codebuild reset bug
  2. Integrate auth & lease login
  3. Write tests and create CI/CD for this repo
  4. Implement remaining functionality. I would like to explore autogenerating dce deploy flags/interactive prompt from variables.tf and API client code/commands from swagger.

@joshmarsh joshmarsh changed the title Feature/create commands Deploy & other basic commands Oct 15, 2019

func (s *LeasesService) LoginToLease(loginAcctID, loginLeaseID string, loginOpenBrowser bool) {
if loginAcctID != "" && loginLeaseID != "" {
log.Println("Please specify either --lease-id or --acctount-id, not both.")
Copy link
Contributor

Choose a reason for hiding this comment

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

The endpoint for this will only be based on the lease ID. I guess we could go the other way too but that isn't the current plan.

Copy link
Contributor

@kddejong kddejong left a comment

Choose a reason for hiding this comment

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

You working on testing?

})

body, _ := ioutil.ReadAll(response.Body)
fmt.Println("Response: ", response)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer the output to only be json. Then I can pipe it our output it more easily.

fmt.Println("Posting to: ", accountsFullURL)
fmt.Println("Post body: ", requestBody)

response := s.Util.Request(&util.ApiRequestInput{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be looking for an err here? What happens if this request fails?

Exit codes should not be 0 and the appropriate error message printed.

}

// Create API request
req, err := http.NewRequest(input.Method, input.Url, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where we are collecting an err we should be testing that its null before continuing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll clean up the error handling and logging once I write some tests for this. I neglected those things since there was a lot of discovery work here and because we've gone back and forth a bit on the requirements for the CLI.

@marinatedpork
Copy link
Contributor

@kddejong, @joshmarsh - Let's call this sufficient as a POC. Please have tests and error handling as a follow up in your coming PR.

@joshmarsh joshmarsh merged commit 7545dcd into master Oct 16, 2019
@joshmarsh joshmarsh deleted the feature/CreateCommands branch October 16, 2019 22:50
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