We had a bad type coercion in the code to update container cluster
resource labels. This fixes the coercion.
We didn't notice this because there was no test exercising the update
code path. I've added the test, which reproduced the panic before this
PR was applied and passes successfully now that the coercion is fixed.
Images now have a licenses field, which lets users specify licenses to
use on an image. But official images already have licenses on them, adn
so Terraform is reading those as a diff. To preserve backwards
compatiblity and avoid a breaking change that would require all
`google_compute_image` users to update their configs, I've set the field
to computed. This means that if no licenses are set in the config, there
can be licenses in the config without prompting a diff. Obviously, this
isn't ideal, as it means you can't ever remove all the licenses from an
image, but I think the benefits here outweigh the drawbacks.
In testing an upcoming `google_compute_region_disk` resource, I had to make these changes. Checking them in separately so that when the magician runs, these changes will already be a part of TF.
Currently, the rolling-update API expects a PUT (regionInstanceGroupManager.update())
instead of a PATCH (regionInstanceGroupManager.patch()) call, so immediate rolling updates
(as specified with `update_strategy = "ROLLING_UPDATE"``) are never triggered:
```
[DEBUG]: ---[ REQUEST ]---------------------------------------
[DEBUG]: POST /compute/beta/projects/mycompany-myapp-staging/regions/europe-west3/instanceGroupManagers/myapp-server/setInstanceTemplate?alt=json HTTP/1.1
....
[DEBUG]: ---[ RESPONSE ]--------------------------------------
...
[DEBUG]: "warnings": [
[DEBUG]: {
[DEBUG]: "code": "FIELD_VALUE_OVERRIDEN",
[DEBUG]: "message": "Update policy type was set to OPPORTUNISTIC. Please use regionInstanceGroupManager.update() to preserve the policy."
[DEBUG]: }
```
refs:
https://cloud.google.com/compute/docs/reference/rest/beta/instanceGroupManagers/patchhttps://cloud.google.com/compute/docs/reference/rest/beta/instanceGroupManagers/updateFix#1506
This was done as its own resource as suggested in slack, since we don't have the option of making all fields Computed in google_compute_instance. There's precedent in the aws provider for this sort of thing (see ami_copy, ami_from_instance).
When I started working on this I assumed I could do it in the compute_instance resource and so I went ahead and reordered the schema to make it easier to work with in the future. Now it's not quite relevant, but I left it in as its own commit that can be looked at separately from the other changes.
Fixes#1582.
When using predefined storage ACLs, you'd get a permadiff, because the
role_entities list was computed, but was never set in state. So it would
be read as empty in the config, and not present in state, so Terraform
would want to pull it down and sync it. This is probably, technically
speaking, a bug in Terraform, but we can work around it by just setting
role_entities to an empty value on every read.
Hypothetically fixes#1643.
@thomasriley, are you able to patch this change into your provider to see if it fixed the problem? I haven't been able to get a working repo so I haven't verified the fix yet.
Move from using qa.test.com, a domain we don't own, to qa.tf-test.club,
a domain we do own, so the domain validation doesn't cause our tests to
fail anymore.
Add a CustomDiff function to storage bucket ACLs that will ignore a diff
if the config and state have the same role_entities, even if they're in
a different order.
Fixes#1525.
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
Added node config 'disk_type' which can either be 'pd-standard' or
'pd-ssd', if left blank 'pd-standard' will be the default used by google
cloud.
Closes: #1656
## What
As well as https://github.com/terraform-providers/terraform-provider-google/pull/1282 , make `resource_container_node_pool` importer accept `{project}/{zone}/{cluster}/{name}` format to specify the project where the node pool belongs to actually.
## Why
Sometimes I want to import container pool in different project from default SA's. However, currently there is no way to specify project the target node pool belongs to, Terraform tries to retrieve node pool from SA's project, then it fails due to `You cannot import non-existent resources using Terraform import.` error.
As discussed in #1326, we're not going to remove name_prefix for
compute_ssl_certificate, because it makes the common use case more
ergonomic by a good amount, and the only cost is it's harder to maintain
the autogenerated code, and we've decided the benefits outweigh the
costs in this circumstance.
* Allow using in repo configuration for cloudbuild trigger
Cloudbuild triggers have a complex configuration that can be defined
from the API. When using the console, the more typical way of doing this
is to defined the configuration within the repository and point the
configuration to the file that defines the config.
This can be supported by sending the filename parameter instead of the
build parameter, however only one can be sent.
* Acceptance testing for cloudbuild trigger with filename
Ensure that when a cloudbuild repo trigger is created with a filename,
that filename is what actually ends up in the cloud.
* Don't specify "by default" in cloudbuild-trigger.
The docs shouldn't say that "cloudbuild.yaml" is used by default. There
is no default from the APIs, but the console suggest using this value.
Just say it's the typical value in documentation.
