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

Add Beta support for allowedPorts field for Cloud Workstations configurations #11299

Merged
merged 3 commits into from
Oct 14, 2024

Conversation

daanheikens
Copy link
Contributor

@daanheikens daanheikens commented Jul 30, 2024

Fixes: hashicorp/terraform-provider-google#18859

Release Note Template for Downstream PRs (will be copied)

workstations: added field `allowedPorts` to `google_workstations_workstation_config`

@daanheikens daanheikens changed the title Add allowedPorts support for workstation config Add Beta support for allowedPorts field for Cloud Workstations configurations Jul 30, 2024
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jul 30, 2024
@daanheikens daanheikens marked this pull request as ready for review July 31, 2024 08:08
@github-actions github-actions bot requested a review from hao-nan-li July 31, 2024 08:09
Copy link

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

@hao-nan-li, 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 modular-magician added service/workstations and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Jul 31, 2024
@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, 87 insertions(+))
google-beta provider: Diff ( 3 files changed, 307 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 43 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 36
Passed tests: 25
Skipped tests: 0
Affected tests: 11

Click here to see the affected service packages
  • workstations

Action taken

Found 11 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_ephemeralDirectories_withSourceImage
  • TestAccWorkstationsWorkstationConfig_update
  • TestAccWorkstationsWorkstationConfig_updateHostDetails
  • TestAccWorkstationsWorkstationConfig_vmTags
  • TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsExample[Debug log]

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamBindingGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamMemberGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_boost[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_ephemeralDirectories_withSourceImage[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_update[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_vmTags[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample[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

@@ -573,6 +580,22 @@ properties:
name: 'disableTcpConnections'
description: |
Disables support for plain TCP connections in the workstation. By default the service supports TCP connections via a websocket relay. Setting this option to true disables that relay, which prevents the usage of services that require plain tcp connections, such as ssh. When enabled, all communication must occur over https or wss.
- !ruby/object:Api::Type::Array
Copy link
Contributor

Choose a reason for hiding this comment

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

It's this top-level field required as well since all inner fields are required. In that case this field needs to be updated in all existing tests for this resource.

Copy link
Contributor Author

@daanheikens daanheikens Aug 5, 2024

Choose a reason for hiding this comment

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

Hi @hao-nan-li, allowed_ports it self is not required, however if defined, first and last are required inside the allowed_ports argument. Please let me know if i configured this correctly!

@github-actions github-actions bot requested a review from hao-nan-li August 5, 2024 11:57
@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Aug 5, 2024
@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, 87 insertions(+))
google-beta provider: Diff ( 4 files changed, 463 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 43 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 37
Passed tests: 26
Skipped tests: 0
Affected tests: 11

Click here to see the affected service packages
  • workstations

Action taken

Found 11 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_ephemeralDirectories_withSourceImage
  • TestAccWorkstationsWorkstationConfig_update
  • TestAccWorkstationsWorkstationConfig_updateHostDetails
  • TestAccWorkstationsWorkstationConfig_vmTags
  • TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsUpdate
  • TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsUpdate[Debug log]

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamBindingGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamMemberGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_boost[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_ephemeralDirectories_withSourceImage[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_update[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_vmTags[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample[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

github-actions bot commented Aug 8, 2024

@hao-nan-li 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

@GoogleCloudPlatform/terraform-team @hao-nan-li This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

Copy link

@GoogleCloudPlatform/terraform-team @hao-nan-li This PR has been waiting for review for 2 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

@hao-nan-li
Copy link
Contributor

Any luck on fixing the tests?

@daanheikens
Copy link
Contributor Author

daanheikens commented Aug 19, 2024

Any luck on fixing the tests?

Hey @hao-nan-li, can you explain why these tests fail? The test i created does not fail correct?

Since it takes quite some time to run these tests, it would help if you could share the errors from the VCR tests. (as they all create their own cluster)

Thanks! Sorry for missing this.

@hao-nan-li
Copy link
Contributor

Any luck on fixing the tests?

Hey @hao-nan-li, can you explain why these tests fail? The test i created does not fail correct?

Since it takes quite some time to run these tests, it would help if you could share the errors from the VCR tests. (as they all create their own cluster)

Thanks! Sorry for missing this.

I think the added field caused some diff, perhaps this field needs to be added to existing tests.

@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Aug 22, 2024
@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, 87 insertions(+))
google-beta provider: Diff ( 4 files changed, 463 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 43 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 37
Passed tests: 5
Skipped tests: 0
Affected tests: 32

Click here to see the affected service packages
  • workstations

Action taken

Found 32 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_basic
  • TestAccWorkstationsWorkstationConfig_boost
  • TestAccWorkstationsWorkstationConfig_disableTcpConnections
  • TestAccWorkstationsWorkstationConfig_displayName
  • TestAccWorkstationsWorkstationConfig_ephemeralDirectories
  • TestAccWorkstationsWorkstationConfig_ephemeralDirectories_withSourceImage
  • TestAccWorkstationsWorkstationConfig_persistentDirectories
  • TestAccWorkstationsWorkstationConfig_readinessChecks
  • TestAccWorkstationsWorkstationConfig_serviceAccount
  • TestAccWorkstationsWorkstationConfig_update
  • TestAccWorkstationsWorkstationConfig_updateHostDetails
  • TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot
  • TestAccWorkstationsWorkstationConfig_updateWorkingDir
  • TestAccWorkstationsWorkstationConfig_vmTags
  • TestAccWorkstationsWorkstationConfig_workstationConfigAcceleratorsExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsUpdate
  • TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigContainerExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigPersistentDirectoriesExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigShieldedInstanceConfigExample
  • TestAccWorkstationsWorkstationConfig_workstationConfigSourceSnapshotExample
  • TestAccWorkstationsWorkstationIamBindingGenerated
  • TestAccWorkstationsWorkstationIamMemberGenerated
  • TestAccWorkstationsWorkstationIamPolicyGenerated
  • TestAccWorkstationsWorkstation_update
  • TestAccWorkstationsWorkstation_workstationBasicExample

Get to know how VCR tests work

@daanheikens
Copy link
Contributor Author

daanheikens commented Aug 22, 2024

Any luck on fixing the tests?

Hey @hao-nan-li, can you explain why these tests fail? The test i created does not fail correct?
Since it takes quite some time to run these tests, it would help if you could share the errors from the VCR tests. (as they all create their own cluster)
Thanks! Sorry for missing this.

I think the added field caused some diff, perhaps this field needs to be added to existing tests.

The field should be optional according to the documentation, which it is right? I thought it might need the default_from_api field but sinceit's optional i wasn't sure. I tried testing myself but i get new errors since i upmerged everything so i cannot verify unfortunately:


Error: Error creating TagKey: googleapi: Error 403: Permission 'resourcemanager.tagKeys.create' denied on resource '//cloudresourcemanager.googleapis.com/organizations/738358245601' (or it may not exist).
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.ErrorInfo",
            "domain": "cloudresourcemanager.googleapis.com",
            "metadata": {
              "permission": "resourcemanager.tagKeys.create",
              "resource": "organizations/[REDACTED]"
            },
            "reason": "IAM_PERMISSION_DENIED"
          }
        ]
        
          with google_tags_tag_key.tag_key1,
          on terraform_plugin_test.tf line 2, in resource "google_tags_tag_key" "tag_key1":
           2: resource "google_tags_tag_key" "tag_key1" {
        
--- FAIL: TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample (1667.75s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-google-beta/google-beta/services/workstations   1668.858s
FAIL
make: *** [testacc] Error 1

@hao-nan-li what should i do next? I never needed these permissions on organization for testing.

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsExample[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigAllowedPortsUpdate[Debug log]

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfigIamBindingGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamMemberGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_basic[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_boost[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_disableTcpConnections[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_displayName[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_ephemeralDirectories[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_ephemeralDirectories_withSourceImage[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_persistentDirectories[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_readinessChecks[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_serviceAccount[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_update[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateWorkingDir[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_vmTags[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigAcceleratorsExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBoostExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigContainerExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigPersistentDirectoriesExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigShieldedInstanceConfigExample[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigSourceSnapshotExample[Error message] [Debug log]
TestAccWorkstationsWorkstationIamBindingGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationIamMemberGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstationIamPolicyGenerated[Error message] [Debug log]
TestAccWorkstationsWorkstation_update[Error message] [Debug log]
TestAccWorkstationsWorkstation_workstationBasicExample[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

@GoogleCloudPlatform/terraform-team @hao-nan-li This PR has been waiting for review for 3 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

Copy link

github-actions bot commented Sep 9, 2024

@GoogleCloudPlatform/terraform-team @hao-nan-li This PR has been waiting for review for 5 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

@daanheikens
Copy link
Contributor Author

I am currently on holiday, will pick this up when I am back.

@c2thorn
Copy link
Member

c2thorn commented Sep 24, 2024

Hi @daanheikens

Tomorrow, the Magic Modules repository is scheduled to undergo a language migration from Ruby to Go. You can view more details about this in our announcement here: hashicorp/terraform-provider-google#19583 (or go/mm-migration-announcement if you are a Googler)

This open pull request may become incompatible due to most YAML and .erb files converting to Go-compatible files.

Our team (Magic Modules repository maintainers) has tooling to automatically convert changes to the new language, and we can prepare a new commit for this pull request that is compatible with the migration.

In order to push the new changes to your pull request, we would need to force push the commit to your fork's branch. Our tooling saves a backup branch before converting, so we could rollback or open a new pull request if needed. We would also work with you and the PR reviewer in the event additional changes are needed.

You also have the option to update the pull request yourself after the migration. You can view a preview branch and updated documentation related to the migration changes.

We will take no action until we have your explicit permission to push changes to your fork's branch used for this pull request. Let me or your reviewer know if you have any further questions!

@daanheikens
Copy link
Contributor Author

Hi @daanheikens

Tomorrow, the Magic Modules repository is scheduled to undergo a language migration from Ruby to Go. You can view more details about this in our announcement here: hashicorp/terraform-provider-google#19583 (or go/mm-migration-announcement if you are a Googler)

This open pull request may become incompatible due to most YAML and .erb files converting to Go-compatible files.

Our team (Magic Modules repository maintainers) has tooling to automatically convert changes to the new language, and we can prepare a new commit for this pull request that is compatible with the migration.

In order to push the new changes to your pull request, we would need to force push the commit to your fork's branch. Our tooling saves a backup branch before converting, so we could rollback or open a new pull request if needed. We would also work with you and the PR reviewer in the event additional changes are needed.

You also have the option to update the pull request yourself after the migration. You can view a preview branch and updated documentation related to the migration changes.

We will take no action until we have your explicit permission to push changes to your fork's branch used for this pull request. Let me or your reviewer know if you have any further questions!

Hi @c2thorn, you can update the branch! All good. Thanks for your message.

Add update tests
Add allowedPorts support for workstation config
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 26, 2024
@tejal29
Copy link
Contributor

tejal29 commented Oct 3, 2024

Thanks folks, Is this ready to merge now?

@daanheikens
Copy link
Contributor Author

Thanks folks, Is this ready to merge now?

Just added my updates. I think this might fixes the last issues. @hao-nan-li can you run the VCR tests again?

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Oct 7, 2024
@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, 87 insertions(+))
google-beta provider: Diff ( 4 files changed, 464 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 43 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 37
Passed tests: 32
Skipped tests: 0
Affected tests: 5

Click here to see the affected service packages
  • workstations

Action taken

Found 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccWorkstationsWorkstationConfig_displayName
  • TestAccWorkstationsWorkstationConfig_update
  • TestAccWorkstationsWorkstationConfig_updateHostDetails
  • TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot
  • TestAccWorkstationsWorkstationConfig_updateWorkingDir

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccWorkstationsWorkstationConfig_displayName[Debug log]
TestAccWorkstationsWorkstationConfig_update[Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Debug log]
TestAccWorkstationsWorkstationConfig_updatePersistentDirectorySourceSnapshot[Debug log]
TestAccWorkstationsWorkstationConfig_updateWorkingDir[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

@daanheikens
Copy link
Contributor Author

@hao-nan-li I think it's fixed, could you review?

@hao-nan-li hao-nan-li merged commit 76e5a0f into GoogleCloudPlatform:main Oct 14, 2024
14 checks passed
gontech pushed a commit to gontech/magic-modules that referenced this pull request Oct 16, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 23, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 24, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Nov 5, 2024
akshat-jindal-nit pushed a commit to akshat-jindal-nit/magic-modules that referenced this pull request Nov 18, 2024
amanMahendroo pushed a commit to amanMahendroo/magic-modules that referenced this pull request Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for allowedPorts field for Cloud Workstations Configurations
5 participants