feature: init version #1
Conversation
| send_user_invitation = each.value.send_user_invitation | ||
|
|
||
| lifecycle { | ||
| ignore_changes = [name] |
There was a problem hiding this comment.
Let's include a comment on why we lifecycle ignore name. I think it's because it gets sets to the users email or something until they first sign in?
There was a problem hiding this comment.
I don't see any naming modifications in our Clients' Datadog. Also, couldn't find any explanation why it was added initially.
I removed this lifecycle, because we can't explain why this is here.
There was a problem hiding this comment.
@gberenice looking good! Few requests, but overall this was thorough and well done!
main.tf
Outdated
| disabled = each.value.disabled | ||
| email = each.value.email | ||
| name = each.value.name | ||
| roles = [data.datadog_role.standard.id] |
There was a problem hiding this comment.
We should work to provide some mapping to the other roles as well -- Just including the standard feels like we're not providing a complete module since we know we've used the others elsewhere. Mind adding that?
There was a problem hiding this comment.
@Gowiem the reason I didn't add the mapping is that currently we use the following schema for access_roles:
- username: veronika.gnilitska
name: Veronika Gnilitska
...
access_roles:
aws: false
datadog: true
So datadog is just enabled or disabled.
I can update this module to support out-of-the-box roles, something like:
access_roles:
datadog:
enabled: true
role: [standard | ro | admin]
And after that - updated our team.yaml.
How does that sound?
There was a problem hiding this comment.
Added support for the out-of-the-box roles. Let me know your thoughts.
There was a problem hiding this comment.
Ah, this access_roles shouldn't be here at all. I'll push some changes soon.
There was a problem hiding this comment.
@Gowiem okay, I think I cleaned this out.
I'd love to work on tests and things like that, let's discuss the priority on our next call.
examples/complete/example.yaml
Outdated
| datadog_api_key: ENC[AES256_GCM,data:joPOPI58VO5E2g==,iv:G3BamrS3ANDMhJqjyZbn1YhAqf2GJ7g7DdVivrYVzDw=,tag:k2sGvVtoxznn7U8x8I7oUw==,type:int] | ||
| datadog_app_key: ENC[AES256_GCM,data:GeerHEomnls2Cg==,iv:9uiNp5vvv/8/6sqNix28FRm1btWy6CF3fnGD3RV3oMI=,tag:LbGRJu7Bz8HdJYR+6iO/vQ==,type:int] |
There was a problem hiding this comment.
Did you create a test datadog account? Because if so... bravo 😁
There was a problem hiding this comment.
Ah, no. We don't have tests yet, so I decided to use a decryptable SOPS file example. SOPS with age doesn't depend on any cloud resource and is good for these types of things.
There was a problem hiding this comment.
I'm all for using age here, makes sense 👍
We should spin up a internal DD org so we can test this module, even if those are tests are manual for now. We could also create some terraform test blocks as well...
Let's talk about this Thursday.
gberenice
force-pushed
the
feature/init_version
branch
from
998e298 to
4f7ce9b
Compare
gberenice
force-pushed
the
feature/init_version
branch
from
3e5c7cc to
8857106
Compare
| validation { | ||
| condition = alltrue([for role in flatten([for user in var.users : user.roles]) : contains(["standard", "admin", "read_only"], role)]) | ||
| error_message = "Each role must be one of 'standard', 'admin', or 'read_only'." | ||
| } |
| for_each = local.users | ||
| disabled = each.value.disabled | ||
| email = each.value.email | ||
| name = each.value.name |
There was a problem hiding this comment.
I think the idea with lifecycle_ignore on the name was that users can change their name in the console and it causes a recreate or drift in the resource. We shouldn't care about how they want to spell their names or how they change them so we lifecycle ignored it... I'm thinking we add that back and make that clear via a comment. Sorry to reverse!
There was a problem hiding this comment.
Ah, okay. I've never seen that users actually care about that 😆 , but it's a valid point.
There was a problem hiding this comment.
Added. Sorry, this has been a bit hectic 😬
There was a problem hiding this comment.
@gberenice hectic? We just very quickly iterated on a bunch of good changes on this PR. This was kinda how I want all PRs to go. Things are going to need to change. We should be bringing up all of our thoughts on what is good practice or what we expect and then having fast feedback loops on iterating on those changes. This was great, hats off to you on doing things quickly and responding correctly to feedback 💯
There was a problem hiding this comment.
@Gowiem thanks for the great feedback!
Info
Datadog Standard Roleand should be extended in the future to handle more cases.