-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add Policy, Attestor, and Note resources for Binary Authorization #1885
Conversation
@ndmckinley in case you want to look at it now, here it is. I found another opportunity to break off some changes separately so I'm going to do that so this isn't crowded with changes to every other resource. |
b2354c9
to
8951bb7
Compare
"github.com/hashicorp/terraform/helper/schema" | ||
) | ||
|
||
func resourceBinaryauthorizationAttestor() *schema.Resource { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fix this capitalization? I did it with a hack for ResourceManager iirc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! I fixed the hack too.
`, name, name, armoredPubKey) | ||
} | ||
|
||
const armoredPubKey = `mQENBFtP0doBCADF+joTiXWKVuP8kJt3fgpBSjT9h8ezMfKA4aXZctYLx5wslWQl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this some particular key, or just a valid one we happen to have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a valid one, I added a comment.
798d263
to
829b531
Compare
All right, I've been fighting with these tests and the magician for the past 2 days now but this is finally ready for review again. Tests are passing. |
return nil, err | ||
} | ||
transformed["id"] = transformedId | ||
transformedAsciiArmoredPgpPublicKey, err := expandBinaryAuthorizationAttestorAttestationAuthorityNotePublicKeysAsciiArmoredPgpPublicKey(original["ascii_armored_pgp_public_key"], d, config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These names get a little crazy, don't they?
}, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"attestation_authority": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole thing is just a single string, right? It's a little messy for users ... is it a lot of work to collapse nested objects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked about this over chat, but for others- yes, it would be a lot of work. Right now we can rename things easily but we don't have an easy way to change the type of a field- the best we'd be able to do is a lot of custom code, which we don't want to do.
f02b49c
to
8872d9d
Compare
…shicorp#1885) <!-- This change is generated by MagicModules. --> /cc @danawillow
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
/cc @danawillow