-
Notifications
You must be signed in to change notification settings - Fork 26
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 resource for adding single projects to shared environment variables #275
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,97 @@ | |||
package vercel_test |
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.
should we test that existing links are not affected?
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.
Not super familiar with Go but everything looks correct to me!
return | ||
} | ||
|
||
response, err := r.client.GetSharedEnvironmentVariable(ctx, state.TeamID.ValueString(), state.SharedEnvironmentVariableID.ValueString()) |
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.
Do we need to handle 404 here, and remove state if the entire shared environment variable is gone?
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.
I don't think so, because if we are Reading it we expect it to exist, and want to detect when it doesn't (e.g. drift) if the shared env var is just gone, then I think that's an error that requires manual intervention rather than trying to recreate the link.
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.
Ahh, okay, because this has to be used with a data source, right?
vercel/resource_shared_environment_variable_project_link_test.go
Outdated
Show resolved
Hide resolved
"team_id": result.TeamID.ValueString(), | ||
"shared_environment_variable_id": result.SharedEnvironmentVariableID.ValueString(), | ||
"project_id": result.ProjectID.ValueString(), | ||
}) |
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.
Another thought - should this have an Import, so you can terraform import team_xxx/env_yyy/prj_zzz ?
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.
hrmmmmmm.. maybe?
I wouldn't really expect to need to import this, and a creation of an existing resource would just be a no-op. I'll be able to test this as I already have projects linked to shared env vars that I would want to put into terraform.
This PR adds a new resource for adding individual projects to a given shared environment variable. This is intended to be used with shared environment variables that are not already terraformed via data source, and usage may conflict with shared environment variables already declared by the shared environment variable resource.