Make google_compute_instance_guest_attributes return empty results when queried for nonexistent values

[karolgorc]

closes hashicorp/terraform-provider-google#20346

• Removed error thrown on Instances.GetGuestAttributes calls
• Added additional Instances.Get call to see if the instance exists at all

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

compute: `google_compute_instance_guest_attributes` will return empty arrays and lists when queried values don't exist instead of throwing an error

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@roaks3, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[github-actions]

@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[roaks3]

Could you add a test to confirm the behavior?

I'm also a little unsure about the solution here. It looks like this data source does not represent a list, so I would generally expect it to follow hashicorp/terraform-provider-google#12873 and return an error. For example, I think that would be the expected behavior when variable_key is used.

However, this datasource has a query_path parameter that makes it a bit weird, and I'm not clear on what behavior we would want. The error the users sees now is obviously not clear, but it seems like technically we would still want an error returned. Could you share more about the use case, and why the user would be purposefully querying for infrastructure that doesn't exist?

[karolgorc]

The error the users sees now is obviously not clear, but it seems like technically we would still want an error returned.

The unclear error is due to the HandleDataSourceNotFound func. I can fix the errors on this resource if that's what we want, and just have clear error messages. In this PR we only throw an error when an instance doesn't exist, but we can use the upstream code and just fix the errors there.

Could you share more about the use case, and why the user would be purposefully querying for infrastructure that doesn't exist?

The guest attributes can only be added via curl on the instance so i'd treat them as a dynamic value that could change rapidly depending on the use case

[github-actions]

@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[roaks3]

Oh I see, I misunderstood the query_value field. This does actually look more like a plural datasource, and I don't think a 404 would make sense when the list is empty.

Same comment as before though, we should be able to add a test with an instance that does not have any guest attributes. This would confirm the error with the old behavior, and that it is fixed with this change.

[karolgorc]

Added test cases for empty and nonexistent scenarios

[github-actions]

@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 90 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 2 files changed, 90 insertions(+), 2 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1077
Passed tests: 1003
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages

• compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccDataSourceComputeInstanceGuestAttributes_basic

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccDataSourceComputeInstanceGuestAttributes_basic [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test