Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set disbaleSsh default value to true for workstation create #11225

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented Jul 22, 2024

fixes hashicorp/terraform-provider-google#19094

Release Note Template for Downstream PRs (will be copied)

workstation: `host.gce_instance.disable_ssh` now defaults to true for `google_workstations_workstation_config`

@github-actions github-actions bot requested a review from c2thorn July 22, 2024 19:24
Copy link

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

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

@modular-magician
Copy link
Collaborator

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-beta provider: Diff ( 1 file changed, 2 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

Breaking Change(s) Detected

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

  • Field host.gce_instance.disable_ssh default value changed from to true on google_workstations_workstation_config - 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
Copy link
Collaborator

Tests analytics

Total tests: 35
Passed tests: 11
Skipped tests: 0
Affected tests: 24

Click here to see the affected service packages
  • workstations

Action taken

Found 24 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccWorkstationsWorkstationConfigIamBindingGenerated
  • TestAccWorkstationsWorkstationConfigIamMemberGenerated
  • TestAccWorkstationsWorkstationConfigIamPolicyGenerated
  • TestAccWorkstationsWorkstationConfig_boost
  • TestAccWorkstationsWorkstationConfig_disableTcpConnections
  • TestAccWorkstationsWorkstationConfig_displayName
  • TestAccWorkstationsWorkstationConfig_readinessChecks
  • TestAccWorkstationsWorkstationConfig_serviceAccount
  • TestAccWorkstationsWorkstationConfig_update
  • TestAccWorkstationsWorkstationConfig_updateHostDetails
  • TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot
  • TestAccWorkstationsWorkstationConfig_updateWorkingDir
  • TestAccWorkstationsWorkstationConfig_vmTags
  • TestAccWorkstationsWorkstationConfig_workstationConfigAcceleratorsExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigContainerExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigPersistentDirectoriesExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigShieldedInstanceConfigExample
  • TestAccWorkstationsWorkstationIamBindingGenerated
  • TestAccWorkstationsWorkstationIamMemberGenerated
  • TestAccWorkstationsWorkstationIamPolicyGenerated
  • TestAccWorkstationsWorkstation_workstationBasicExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamBindingGenerated[Debug log]
TestAccWorkstationsWorkstationConfigIamMemberGenerated[Debug log]
TestAccWorkstationsWorkstationConfig_disableTcpConnections[Debug log]
TestAccWorkstationsWorkstationConfig_displayName[Debug log]
TestAccWorkstationsWorkstationConfig_serviceAccount[Debug log]
TestAccWorkstationsWorkstationConfig_update[Debug log]
TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot[Debug log]
TestAccWorkstationsWorkstationConfig_updateWorkingDir[Debug log]
TestAccWorkstationsWorkstationConfig_vmTags[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigAcceleratorsExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigContainerExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigPersistentDirectoriesExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigShieldedInstanceConfigExample[Debug log]
TestAccWorkstationsWorkstationIamBindingGenerated[Debug log]
TestAccWorkstationsWorkstationIamMemberGenerated[Debug log]
TestAccWorkstationsWorkstationIamPolicyGenerated[Debug log]
TestAccWorkstationsWorkstation_workstationBasicExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_boost[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_readinessChecks[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

Copy link
Member

@c2thorn c2thorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The errors look intermittent, but adding a default value to an existing field is a breaking change. This can happen in the 6.0.0 branch that we have incoming for the next major release, would you like to move the change there?

Also, is there an associated Github issue and/or buganizer ticket for this change?

@tejal29
Copy link
Contributor Author

tejal29 commented Jul 23, 2024

The errors look intermittent, but adding a default value to an existing field is a breaking change. This can happen in the 6.0.0 branch that we have incoming for the next major release, would you like to move the change there?

Also, is there an associated Github issue and/or buganizer ticket for this change?

sounds good. I can move this PR there.
There is a buganizer issue. Is it safe to link it here?

Is this the 6.0.0 branch? https://github.com/GoogleCloudPlatform/magic-modules/tree/FEATURE-BRANCH-major-release-6.0.0

@github-actions github-actions bot requested a review from c2thorn July 23, 2024 20:43
@c2thorn
Copy link
Member

c2thorn commented Jul 24, 2024

sounds good. I can move this PR there. There is a buganizer issue. Is it safe to link it here?

yes, shortened b/ links are posted in our Github issues all the time

Is this the 6.0.0 branch? FEATURE-BRANCH-major-release-6.0.0

That is correct, I can change the base branch in this PR myself. Let me do so

@c2thorn c2thorn changed the base branch from main to FEATURE-BRANCH-major-release-5.0.0 July 24, 2024 20:30
@c2thorn c2thorn changed the base branch from FEATURE-BRANCH-major-release-5.0.0 to FEATURE-BRANCH-major-release-6.0.0 July 24, 2024 20:30
@c2thorn
Copy link
Member

c2thorn commented Jul 25, 2024

/gcbrun

@modular-magician
Copy link
Collaborator

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-beta provider: Diff ( 1 file changed, 2 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

Breaking Change(s) Detected

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

  • Field host.gce_instance.disable_ssh default value changed from to true on google_workstations_workstation_config - 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
Copy link
Collaborator

Tests analytics

Total tests: 35
Passed tests: 31
Skipped tests: 0
Affected tests: 4

Click here to see the affected service packages
  • workstations

Action taken

Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccWorkstationsWorkstationConfigIamPolicyGenerated
  • TestAccWorkstationsWorkstationConfig_boost
  • TestAccWorkstationsWorkstationConfig_readinessChecks
  • TestAccWorkstationsWorkstationConfig_updateHostDetails

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Debug log]
TestAccWorkstationsWorkstationConfig_boost[Debug log]
TestAccWorkstationsWorkstationConfig_readinessChecks[Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

Copy link

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

Copy link
Member

@c2thorn c2thorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

github-actions bot commented Aug 9, 2024

@tejal29, this PR is waiting for action from you. Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

Image showing the re-request review button

If no action is taken, this PR will be closed in 28 days.

This notification can be disabled with the disable-automatic-closure label.

@tejal29 tejal29 force-pushed the set_disable_ssh_true branch from 59a4a3a to cfc7633 Compare August 9, 2024 18:32
@github-actions github-actions bot requested a review from c2thorn August 9, 2024 18:32
@modular-magician
Copy link
Collaborator

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 ( 1 file changed, 6 insertions(+))
google-beta provider: Diff ( 2 files changed, 8 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

Breaking Change(s) Detected

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

  • Field host.gce_instance.disable_ssh default value changed from to true on google_workstations_workstation_config - 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
Copy link
Collaborator

Tests analytics

Total tests: 35
Passed tests: 35
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • workstations

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@github-actions github-actions bot requested a review from c2thorn August 12, 2024 16:32
@modular-magician
Copy link
Collaborator

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 ( 1 file changed, 6 insertions(+))
google-beta provider: Diff ( 2 files changed, 8 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

Breaking Change(s) Detected

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

  • Field host.gce_instance.disable_ssh default value changed from to true on google_workstations_workstation_config - 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
Copy link
Collaborator

Tests analytics

Total tests: 35
Passed tests: 35
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • workstations

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@c2thorn c2thorn added the override-breaking-change Allows a potential breaking change to be merged label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
override-breaking-change Allows a potential breaking change to be merged service/workstations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants