fix(tags): import of tag resources failed with 403 error #620
Conversation
nlevee
requested review from
ulucinar,
sergenyalcin and
turkenf
as code owners
nlevee
force-pushed
the
fix/tag-import-not-found
branch
2 times, most recently
from
ad48e56
to
a8fe9e3
Compare
Signed-off-by: Nicolas Levée <[email protected]>
nlevee
force-pushed
the
fix/tag-import-not-found
branch
from
a8fe9e3
to
4170db9
Compare
|
Hi! Is there something wrong ? Please can you check if I have to rework something 🙏 |
|
Hi @ulucinar @sergenyalcin @turkenf, Is there anything I can do to pass this PR ? |
|
@nlevee We're aiming for a new provider release next week and will look to get this PR in for it. |
|
/test-examples="examples/tags/v1beta1/tagkey.yaml" |
|
/test-examples="examples/tags/v1beta1/tagvalue.yaml" |
|
/test-examples="examples/tags/v1beta1/tagbinding.yaml" |
Done 👍 thank you |
| @@ -19,15 +19,18 @@ func Configure(p *config.Provider) { | |||
| TerraformName: "google_tags_tag_value", | |||
| Extractor: common.ExtractResourceIDFuncPath, | |||
| } | |||
| config.MarkAsRequired(r.TerraformResource, "parent", "tag_value") | |||
There was a problem hiding this comment.
Why do we mark these fields as required? and for the other two resources?
There was a problem hiding this comment.
Because those fileds are required in GCP API to retrieve and set.
| apiVersion: tags.gcp.upbound.io/v1beta1 | ||
| kind: TagBinding | ||
| metadata: | ||
| annotations: | ||
| meta.upbound.io/example-id: tags/v1beta1/tagbinding | ||
| upjet.upbound.io/manual-intervention: "The resource requires a real external name" | ||
| crossplane.io/external-name: "&{tagBindingID}" | ||
| labels: | ||
| testing.upbound.io/example-name: binding | ||
| name: binding-external | ||
| spec: | ||
| managementPolicies: ["Observe"] | ||
| forProvider: {} | ||
|
|
||
| --- |
There was a problem hiding this comment.
We do not prefer to have examples of the same kind in the same YAML file. If the example you added is for a different scenario, for example, observe, it would be better to add a new example named tagbinding-observe.yaml.
There was a problem hiding this comment.
@nlevee, I have successfully tested the import of an existing resource with the example you gave, but when I try to create the resources from scratch, I see the following error. Can you create the resources successfully?
conditions:
- lastTransitionTime: "2024-11-01T15:05:53Z"
message: 'observe failed: failed to observe the resource: [{0 Error when reading
or editing TagsTagKey "key": googleapi: Error 400: com.google.apps.framework.request.StatusException:
<eye3 title=''INVALID_ARGUMENT''/> generic::INVALID_ARGUMENT: Invalid CRM resource
name: ''tagKeys/key''. []}]'
reason: ReconcileError
status: "False"
type: Synced
I'll try to reproduce on my side, I'll give you feed back asap. |
Signed-off-by: Fatih Türken <[email protected]>
|
/test-examples="examples/tags/v1beta1/tagkey.yaml" |
|
I added data source for project id to the examples and removed the manual intervention, you can see the error in the uptest run.
|
nlevee
force-pushed
the
fix/tag-import-not-found
branch
from
87f0581
to
494d0cb
Compare
When it create the resource, it must not set the label I think, I misconfigured it in |
|
Hi @turkenf, can you help me ? 🙏 I realy don't understand why it try to get the resources before create... The external-name is not predictable. |
|
Hi @turkenf 👋, |
|
Hi @nlevee, Thank you for your patience here; I will check it out when I can and let you know. |
Description of your changes
I've changed the way tags are imported when “managementPolicies” is set to “Observe”.
Currently, the import fails with at best a 403 error.
The problem is that the Terraform provider's import function expects to receive the “name” as an argument, with only the name as the value (without the “tagXXX/” prefix).
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
Tested with resources in example directory.
Link to #627