service_account_key: regression fix for v1.14 #1664
Conversation
| project, err := getProject(d, config) | ||
| // in the provider configuration. If neither exist use an empty project | ||
| // when building the service account FQN | ||
| project, _ := getProject(d, config) |
There was a problem hiding this comment.
Generally I'm not too into the idea of ignoring errors. What if serviceAccountFQN took d and config as parameters, and then it could do the getProject call at the time where there does actually need to be a project set?
There was a problem hiding this comment.
That seems much better, I'll do that!
There was a problem hiding this comment.
@danawillow I have made the requested changes, and it looks much cleaner now, thanks :)
vishen
force-pushed
the
service_account_key_regression_fix
branch
from
5ac6ab0 to
5b27002
Compare
| // Get the project from the resource or fallback to the project | ||
| // in the provider configuration | ||
| project, err := getProject(d, config) | ||
| serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), d, config) |
There was a problem hiding this comment.
Looking at the diff, I think this one needs to be just account_id
google/utils.go
Outdated
| } | ||
| // "projects/(-|<project>)/serviceAccounts/<service_account_id>@<project>.iam.gserviceaccount.com" | ||
| // A project is required if we are trying to build the FQN from a service account id and | ||
| // and error will be returned in this case |
There was a problem hiding this comment.
"in this case" -> let's clarify that the error will be returned if no project is set in either the passed in service account or the provider-level config
Commit 8f31fec introduced a bug for the 'service_account_key' resource where it required a project be set either in the provider or in the resource for 'service_account_key', but a project isn't required if the service account is a service account fully qualified name or a service account email. This PR relaxes the requirement that a project needs to be set for the 'service_account_key' resource, 'service_account' datasource and 'service_account_key' datasource, but will error if we try to build a fully qualified name from a service account id when no project can be found. This also cleans up 'serviceAccountFQN' so it is slightly easier to follow and return an error if there is no project but we need one to build the service account fully qualified name. Fixes: hashicorp#1655
vishen
force-pushed
the
service_account_key_regression_fix
branch
from
5b27002 to
38b28a2
Compare
|
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 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Commit 8f31fec introduced a bug for the 'service_account_key' resource
where it required a project be set either in the provider or in the
resource for 'service_account_key', but a project isn't required if the
service account is a service account fully qualified name or a service
account email.
This PR relaxes the requirement that a project needs to be set for the
'service_account_key' resource, 'service_account' datasource and
'service_account_key' datasource, but will error if we try to build a
fully qualified name from a service account id when no project can be
found.
This also cleans up 'serviceAccountFQN' so it is slightly easier to
follow and return an error if there is no project but we need one to
build the service account fully qualified name.
Fixes: #1655