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.

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

Static Credential Store #236

Merged
merged 12 commits into from
Jul 27, 2022
Merged

Static Credential Store #236

merged 12 commits into from
Jul 27, 2022

Conversation

kheina
Copy link
Contributor

@kheina kheina commented Jul 21, 2022 •
edited
Loading

adds static credential store to the terraform boundary provider

including

  • automated tests
    • automated tests passing
  • manual testing
  • examples

testing details

  • added below tf snippet, confirmed static credential store created
  • removed below tf snipped, confirmed static credential store destroyed
resource "boundary_credential_store_static" "example" {
  name        = "example_static_credential_store"
  description = "this is a static credential store being created for testing purposes only."
  scope_id    = boundary_scope.project.id
}

boundary credential-stores list -scope-id <scope_id>

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 21, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

johanbrandhorst
johanbrandhorst requested changes Jul 25, 2022
View reviewed changes
internal/provider/provider.go Outdated Show resolved Hide resolved
internal/provider/resource_credential_store_static_test.go Outdated Show resolved Hide resolved
internal/provider/resource_scope_test.go Outdated
@@ -42,6 +42,7 @@ resource "boundary_scope" "proj1" {
scope_id = boundary_scope.org1.id
description = "foo"
depends_on = [boundary_role.org1_admin]
auto_create_admin_role = true // this is necessary for creating static credential store. without it, boundary returns a 403 on creation
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be interested in hearing Louis' thoughts on this

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this has to do with not setting the depends_on in the static resource, would be curious if that resolves it.

louisruch
louisruch reviewed Jul 26, 2022
View reviewed changes
Copy link
Contributor

@louisruch louisruch left a comment

Choose a reason for hiding this comment

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

I believe adding the depends_on will remove the scope changes you added. Otherwise this looks perfect! Well done

johanbrandhorst
johanbrandhorst approved these changes Jul 27, 2022
View reviewed changes
Copy link
Contributor

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Lets ship it!

louisruch
louisruch approved these changes Jul 27, 2022
View reviewed changes
Copy link
Contributor

@louisruch louisruch left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@kheina kheina merged commit 48672fd into main Jul 27, 2022
@kheina kheina deleted the daniellemiu_add-static-cred-store branch July 27, 2022 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@louisruch louisruch louisruch approved these changes

@johanbrandhorst johanbrandhorst johanbrandhorst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kheina @hashicorp-cla @johanbrandhorst @louisruch