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

Team: Support org_id attribute #979

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Conversation

julienduchesne
Copy link
Member

@julienduchesne julienduchesne commented Jul 13, 2023

Closes #747
Final resource to receive the org_id
Allows a team to be created and managed in a separate organization without needing two provider blocks
Also adds tests that a team can be created and managed in an org

Note: *_permissions are a bit messy. I'm planning on refactoring them afterwards, they mostly follow the same pattern so it should be possible to make them much nicer

@github-actions
Copy link

In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically. To do so, a Grafana Labs employee must promote the Drone build.

For maintainers, it's better to run only the Cloud tests you need, rather than all of them. You can do so by setting the following parameter when promoting:

TESTARGS='-run=<testname>'

@julienduchesne julienduchesne force-pushed the julienduchesne/team-org-id branch 2 times, most recently from e440dae to e87729c Compare July 14, 2023 12:44
@julienduchesne julienduchesne marked this pull request as ready for review July 14, 2023 12:49
@julienduchesne julienduchesne requested a review from a team as a code owner July 14, 2023 12:49
Copy link
Contributor

@inkel inkel left a comment

Choose a reason for hiding this comment

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

It looks good to me, but I have one question that I'd like to understand before moving on.

@@ -50,6 +51,12 @@ func ResourceDashboardPermission() *schema.Resource {
Type: schema.TypeSet,
Required: true,
Description: "The permission items to add/update. Items that are omitted from the list will be removed.",
// Ignore the org ID of the team when hashing. It works with or without it.
Copy link
Contributor

Choose a reason for hiding this comment

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

If it works with or without it, why ignoring it then? Not sure I follow 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

This could be explained better in the comment. The reason why the org ID can be ignored is that resources that refer to each other have to be in the same org anyways (except global resources like a user). So if you specify a team that is not within the same org as the permissions, the API will fail.

And the reason why it HAS to be ignored with the set function is that 1:1 (AKA: team 1 in org 1) is functionally equivalent to team 1 (that is team 1 in an unspecified org). Lots of this is due to migration, and can definitely be improved once we deprecate the global org setting. At that point, all of the team IDs should always have the org ID in them and we can stop doing weird exceptions like this

@julienduchesne julienduchesne force-pushed the julienduchesne/team-org-id branch 2 times, most recently from 6530b87 to 0fd8b11 Compare July 14, 2023 14:18
Closes #747
Final resource to receive the `org_id`
Allows a team to be created and managed in a separate organization without needing two provider blocks
Also adds tests that a team can be created and managed in an org

Note: `*_permissions` are a bit messy. I'm planning on refactoring them into blocks for their respective resources, so I'm not going to clean them up too much right now.
@julienduchesne julienduchesne merged commit e3c8585 into master Jul 14, 2023
3 checks passed
@julienduchesne julienduchesne deleted the julienduchesne/team-org-id branch July 14, 2023 14:49
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.

Support Setting of org_id outside of provider block
2 participants