fix: (storagetransfer) transfer path can be changed to empty string

[gurusai-voleti]

Release Note Template for Downstream PRs (will be copied) Fixes: https://github.com/hashicorp/terraform-provider-google/issues/13673 See Write release notes for guidance.

storagetransfer: fix to allow empty string for path

[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 ( 3 files changed, 206 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 206 insertions(+), 7 deletions(-))

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 15 Skipped tests: 0 Affected tests: 1

Click here to see the affected service packages

• storagetransfer

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

• TestAccStorageTransferJob_transferPathError

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode: TestAccStorageTransferJob_transferPathError [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

[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 ( 3 files changed, 205 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 205 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

[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 ( 3 files changed, 205 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 205 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 16 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages

• storagetransfer

🟢 All tests passed!

View the build log

[github-actions[bot]]

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

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@ScottSuarez, 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.

[kautikdk]

Hi @gurusai-voleti, what is the solution that you are proposing here?

[ScottSuarez]

This appears to be a breaking change, see https://github.com/GoogleCloudPlatform/magic-modules/pull/14300#issuecomment-2984189491

[gurusai-voleti]

Hi @gurusai-voleti, what is the solution that you are proposing here?

firstly the field is missing validation, added validation to the path field as gcs object path cannot be started with "/" (reference here https://cloud.google.com/storage-transfer/docs/reference/rest/v1/GcsData) and field is an user provided and cannot be marked as computed so changed path to optional field only

as the customer reported issue they used "/" first and later realized to change to "" empty string which they cannot as in update d.HasChanges will not detect empty string as a change and an empty string cannot be considered for computed field as per terraform sdk (error occurred with computed set to true A computed value with the empty string as the new value and a non-empty old value was found. Interpreting the empty string as "unset" to align with legacy behavior.) so tf plan/apply will show no changes required, so removed computed for the field and as it is user provided no need of computed as well

as d.HasChanges and computed cannot consider empty string as a change or input, added DiffSuppressFunc if the old and new is same it will not show any difference in tf plan, if its different it will show difference in tf plan and path can be updated to "" empty string

[gurusai-voleti]

This appears to be a breaking change, see #14300 (comment)

yes it's a breaking change, so created an PR on top 7.0 feature branch, Anything else needs to be taken care?

[kautikdk]

If the field is currently Computed, do we know why it was marked as such? i.e Was there any change from the API side which mandated this field to get server side value as well if not provided by user?

[gurusai-voleti]

If the field is currently Computed, do we know why it was marked as such? i.e Was there any change from the API side which mandated this field to get server side value as well if not provided by user?

even if its computed or not, as its optional field if API response has value, it can be set otherwise not

[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 ( 3 files changed, 123 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 123 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Errors

google-beta provider:

• Failed to detect subcategory in the frontmatter in file website/docs/guides/version_7_upgrade.html.markdown.

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 15 Skipped tests: 0 Affected tests: 1

Click here to see the affected service packages

• storagetransfer

#### 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

• TestAccStorageTransferJob_transferUpdateToEmptyString

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode: TestAccStorageTransferJob_transferUpdateToEmptyString [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

[ScottSuarez]

This appears to be a breaking change, see #14300 (comment)

yes it's a breaking change, so created an PR on top 7.0 feature branch, Anything else needs to be taken care?

oops, I didn't realize we opened for changes to the major release branch. Thanks for that !!

[ScottSuarez]

Looks like merging main into this branch has added unintended changes here.

[gurusai-voleti]

Looks like merging main into this branch has added unintended changes here.

git rebase went wrong

[ScottSuarez]

If the field is currently Computed, do we know why it was marked as such? i.e Was there any change from the API side which mandated this field to get server side value as well if not provided by user?

even if its computed or not, as its optional field if API response has value, it can be set otherwise not

Do we have understanding of when/in what scenario server sets a value for this field independent of user input? While yes we can switch to optional here, I am looking to see how disruptive this might be for the user.

If the user needs to deploy then define this field post deployment because the server returned some value this is not an optimal experience. In such a case we should likely explicitly set the value to empty string if valid to avoid two step apply. As long as such a change isn't very divergent from normal user flow.

[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 ( 3 files changed, 205 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 205 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Errors

google-beta provider:

• Failed to detect subcategory in the frontmatter in file website/docs/guides/version_7_upgrade.html.markdown.

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 16 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages

• storagetransfer

🟢 All tests passed!

View the build log

[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 ( 3 files changed, 205 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 205 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Errors

google-beta provider:

• Failed to detect subcategory in the frontmatter in file website/docs/guides/version_7_upgrade.html.markdown.

[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 ( 3 files changed, 123 insertions(+), 7 deletions(-)) google-beta provider: Diff ( 3 files changed, 123 insertions(+), 7 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

• Field replication_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field replication_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_sink.path transitioned from optional+computed to optional google_storage_transfer_job - reference
• Field transfer_spec.gcs_data_source.path transitioned from optional+computed to optional google_storage_transfer_job - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Errors

google-beta provider:

• Failed to detect subcategory in the frontmatter in file website/docs/guides/version_7_upgrade.html.markdown.

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 16 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages

• storagetransfer

🟢 All tests passed!

View the build log

[modular-magician]

Tests analytics

Total tests: 16 Passed tests: 16 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages

• storagetransfer

🟢 All tests passed!

View the build log

[gurusai-voleti]

this branch has been corrupted created a new PR here https://github.com/GoogleCloudPlatform/magic-modules/pull/14377, please check the new PR @kautikdk @ScottSuarez please add yourself as a reviewer for new PR

[gurusai-voleti]

why it was marke

If the field is currently Computed, do we know why it was marked as such? i.e Was there any change from the API side which mandated this field to get server side value as well if not provided by user?

even if its computed or not, as its optional field if API response has value, it can be set otherwise not

Do we have understanding of when/in what scenario server sets a value for this field independent of user input? While yes we can switch to optional here, I am looking to see how disruptive this might be for the user.

If the user needs to deploy then define this field post deployment because the server returned some value this is not an optimal experience. In such a case we should likely explicitly set the value to empty string if valid to avoid two step apply. As long as such a change isn't very divergent from normal user flow.

if the user doesn't provide any value, API doesn't set any value, it just reflects what user provided