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

update external_ipv6_prefix to not be output only #9096

Merged
merged 9 commits into from
Nov 9, 2023

Conversation

BBBmau
Copy link
Collaborator

@BBBmau BBBmau commented Sep 26, 2023

This addresses the resource google_compute_subnetwork where the attribute externalIpv6Prefix is set as Ouput only, however when looking at the Google Docs it's not an output attribute. It's also worth mentioning that it has a Default value of "2600:1900:4000:1ae::/64" which is not mentioned on the Google Cloud docs

Release Note Template for Downstream PRs (will be copied)

compute: changed `external_ipv6_prefix` field on `google_compute_subnetwork` resource to not be output only

@modular-magician
Copy link
Collaborator

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will require approval to run.

@NickElliot, 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 awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Sep 26, 2023
@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.

Terraform GA: Diff ( 2 files changed, 21 insertions(+), 8 deletions(-))
Terraform Beta: Diff ( 2 files changed, 21 insertions(+), 8 deletions(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3089
Passed tests 2789
Skipped tests: 299
Affected tests: 1

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
TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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

Rerun these tests in REPLAYING mode to catch issues

$\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
Contributor

@NickElliot NickElliot left a comment

Choose a reason for hiding this comment

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

Could you include the external_ipv6_prefix in the update tests to verify the functionality is stable if update support is being added for it? Thanks!

@BBBmau
Copy link
Collaborator Author

BBBmau commented Oct 6, 2023

Been attempting to add it to the update function but im finding this issue that was not present before. Could use some input on this since i would've expected the same error to appear when updating the resource with the other configurations.

=== CONT  TestAccComputeSubnetwork_update
    vcr_utils.go:152: Step 5/6 error: Error running apply: exit status 1
        
        Error: Error updating Subnetwork "projects/hc-terraform-testing/regions/us-central1/subnetworks/tf-test-4ljmmmy06d": googleapi: Error 400: Required field 'resource.fingerprint' not specified, required
        
          with google_compute_subnetwork.network-with-private-google-access,
          on terraform_plugin_test.tf line 7, in resource "google_compute_subnetwork" "network-with-private-google-access":
           7: resource "google_compute_subnetwork" "network-with-private-google-access" {
        
--- FAIL: TestAccComputeSubnetwork_update (198.07s)

docs says that the fingerprint attribute is required when updating the resource, which is odd since its not used at all.
https://cloud.google.com/compute/docs/reference/rest/v1/subnetworks#:~:text=Fingerprint%20of%20this,base64%2Dencoded%20string.

@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Oct 17, 2023
@BBBmau
Copy link
Collaborator Author

BBBmau commented Oct 17, 2023

I'll need some more input on this, I went ahead and manually added the external_ipv6 attribute in update3 and found this error. Though I am unsure what format is expecting since this is the default value

│ Error: Error creating Subnetwork: googleapi: Error 400: Invalid value for field 'resource.externalIpv6Prefix': '2600:1900:41a0:4ec9:0:0:0:0/64'. Specified external IPv6 address is not allocated to the project or does not belong to the specified scope., invalid

I also attempted to use google_compute_global_address with it in hopes that it would solve the error but no luck with that.

@NickElliot
Copy link
Contributor

Could you push the test config you were using with global address? Thanks

@modular-magician modular-magician removed the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Oct 18, 2023
@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.

Terraform GA: Diff ( 3 files changed, 59 insertions(+), 8 deletions(-))
Terraform Beta: Diff ( 3 files changed, 59 insertions(+), 8 deletions(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3161
Passed tests 2842
Skipped tests: 318
Affected tests: 1

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
TestAccComputeSubnetwork_update

Get to know how VCR tests work

@BBBmau
Copy link
Collaborator Author

BBBmau commented Oct 18, 2023

sure thing @NickElliot

provider "google" {
  zone = "us-west1-a"
}

resource "google_compute_network" "custom-test" {
  project = "hc-terraform-testing"
  name                    = "test"
  auto_create_subnetworks = false
}

resource "google_compute_global_address" "ipv6" {
  project = "hc-terraform-testing"
  name = "prefix"
  ip_version = "IPV6"
}

resource "google_compute_subnetwork" "subnetwork" {
  project = "hc-terraform-testing"
  name          = "testtest"
  ip_cidr_range    = "10.0.0.0/16"
  region           = "us-central1"
  network          = google_compute_network.custom-test.self_link
  stack_type       = "IPV4_IPV6"
  ipv6_access_type = "EXTERNAL"
  external_ipv6_prefix = "2600:1900:41a0:4ec9:0:0:0:0/64"
}

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeSubnetwork_update[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Oct 20, 2023
@modular-magician modular-magician removed the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Oct 20, 2023
@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.

Terraform GA: Diff ( 2 files changed, 21 insertions(+), 8 deletions(-))
Terraform Beta: Diff ( 2 files changed, 21 insertions(+), 8 deletions(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3164
Passed tests 2846
Skipped tests: 317
Affected tests: 1

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
TestAccDataprocJobIamPolicy

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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

Rerun these tests in REPLAYING mode to catch issues

$\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

@rileykarson
Copy link
Member

rileykarson commented Oct 23, 2023

Provider branch: https://github.com/hashicorp/terraform-provider-google-beta/commits/external-ipv6-prefix
Parametised config: https://gist.github.com/rileykarson/90433ce0ecae8e83bbdad837c48987e3

Build the provider and add it to a local cache:

  1. git clone https://github.com/hashicorp/terraform-provider-google-beta.git
  2. cd terraform-provider-google-beta
  3. git checkout external-ipv6-prefix
  4. make build && go build
  5. mkdir -p $HOME/.terraform.d/plugins/googleapis.com/google/google-local/1.0.0/ARCHITECTURE/terraform-provider-google-local
  6. mv terraform-provider-google-beta $HOME/.terraform.d/plugins/googleapis.com/google/google-local/1.0.0/ARCHITECTURE/terraform-provider-google-local
  7. Add a main.tf file with these contents: https://gist.github.com/rileykarson/beef76d10ca4c4410b7f6f1f37d2d866

Given a project and an available range (a subset of the overall range):

  1. Run terraform apply supplying the project and a subset of the range
  2. Run terraform apply supplying the project and the complete range

Capture the entire terminal output between the first command until the end of the second, and share privately over email with Google/HashiCorp stakeholders.

@SarahFrench SarahFrench changed the title update externalIpv6Prefix to not be output only update external_ipv6_prefix to not be output only Oct 31, 2023
Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Validated offline

@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 hasn't generated any diffs, but I'll let you know if a future commit does.

swamitagupta pushed a commit to swamitagupta/magic-modules that referenced this pull request Nov 14, 2023
…orm#9096)

* update externalIpv6Prefex to not be output only

* WIP: adding attribute to update tests

* remove tests

* remove update4 config

* Update mmv1/products/compute/Subnetwork.yaml

Co-authored-by: Riley Karson <[email protected]>

* add back internalipv6 test

* add endline

* fix lint error

---------

Co-authored-by: Riley Karson <[email protected]>
trodge pushed a commit to trodge/magic-modules that referenced this pull request Nov 27, 2023
…orm#9096)

* update externalIpv6Prefex to not be output only

* WIP: adding attribute to update tests

* remove tests

* remove update4 config

* Update mmv1/products/compute/Subnetwork.yaml

Co-authored-by: Riley Karson <[email protected]>

* add back internalipv6 test

* add endline

* fix lint error

---------

Co-authored-by: Riley Karson <[email protected]>
BBBmau added a commit to BBBmau/magic-modules that referenced this pull request Nov 28, 2023
…orm#9096)

* update externalIpv6Prefex to not be output only

* WIP: adding attribute to update tests

* remove tests

* remove update4 config

* Update mmv1/products/compute/Subnetwork.yaml

Co-authored-by: Riley Karson <[email protected]>

* add back internalipv6 test

* add endline

* fix lint error

---------

Co-authored-by: Riley Karson <[email protected]>
jialei-chen pushed a commit to jialei-chen/magic-modules that referenced this pull request Nov 29, 2023
…orm#9096)

* update externalIpv6Prefex to not be output only

* WIP: adding attribute to update tests

* remove tests

* remove update4 config

* Update mmv1/products/compute/Subnetwork.yaml

Co-authored-by: Riley Karson <[email protected]>

* add back internalipv6 test

* add endline

* fix lint error

---------

Co-authored-by: Riley Karson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants