magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

provider: eliminated the need to manually add *_wo and *_wo_version fields

Open ramonvermeulen opened this issue 6 months ago • 1 comments

Closes https://github.com/hashicorp/terraform-provider-google/issues/23213

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

provider: eliminated the need to manually add *_wo and *_wo_version fields

ramonvermeulen avatar Jun 10 '25 19:06 ramonvermeulen

@BBBmau @melinath

FYI: Just a quick update since returning from vacation and resuming work on this refactor.

I am making some progress toward generating the desired code, though currently encountering some issues with slice properties being generated as empty slices (e.g., RequiredWith, ConflictsWith and ExactlyOneOf). Wrapping up for now, but I plan to set up a local debugging environment later this week to investigate further (would be nice to set breakpoints).

If anyone is interested in reviewing or investigating this in the meantime, I've added some comments and log statements to help clarify the issue.

EDIT: Slice properties seem to be generated now as well, issue was that the values need to be snake_case terraform resource access paths, and it needs to work for nested write-only fields as well. Will later on look into alternatives to:

  • the camelToSnake (maybe there is something in the API)
  • the requiredWith = parent.TerraformLineage() + ".0." + writeOnlyField.TerraformLineage() examples feels a bit "ducktapy"

At least for the current resources the generated code looks close to what it should be:

resource_bigquery_data_transfer_config.go image

resource_secret_manager_secret_version.go image

ramonvermeulen avatar Jun 17 '25 20:06 ramonvermeulen

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.

@zli82016, 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[bot] avatar Jun 24 '25 21:06 github-actions[bot]

@modular-magician reassign-reviewer @BBBmau

melinath avatar Jun 24 '25 21:06 melinath

I know it isn't 100% done yet, but would like to get a first review in on what I have so far.

The main question I have at the moment is if there is a way to get the full field path for a *Type instance, e.g. http_check.0.auth_info.0.password via any of the existing methods. For the issue I am currently facing with the buildPath method something like this is required to set properties such as Conflicts, RequiredWith, ExactlyOneOf in a correct manner. Otherwise I might have to build something custom for this, keeping track of the nesting depth constructing a key during the applied recursion for nested properties.

Also I would love to hear your opinion on mark_write_only_version_mutable, wondering if there are better ways of not introducing a breaking change. If we decide to take this approach, we must ensure removing this field doesn't get forgotten when moving to the next major (7.0.0).

Another thing I noticed is that this might introduce a second breaking change (TypeString will change to TypeInt): https://github.com/GoogleCloudPlatform/magic-modules/blob/7de4ff38ee8db85e6059ec6270531b7b4385e4ba/mmv1/products/monitoring/UptimeCheckConfig.yaml#L264

ramonvermeulen avatar Jun 24 '25 21:06 ramonvermeulen

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 ( 8 files changed, 84 insertions(+), 72 deletions(-)) google-beta provider: Diff ( 8 files changed, 84 insertions(+), 72 deletions(-)) terraform-google-conversion: Diff ( 1 file changed, 10 insertions(+), 10 deletions(-))

Breaking Change(s) Detected

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

  • Field http_check.auth_info.password_wo_version changed from TypeString to TypeInt on google_monitoring_uptime_check_config - reference
  • Field http_check.auth_info.password_wo_version default value changed from <nil> to 0 on google_monitoring_uptime_check_config - reference
  • Field sensitive_params.secret_access_key_wo_version default value changed from <nil> to 0 on google_bigquery_data_transfer_config - reference
  • Field sensitive_params.secret_access_key_wo within resource google_bigquery_data_transfer_config was added to exactly one of - reference
  • Field sensitive_params.secret_access_key within resource google_bigquery_data_transfer_config was added to exactly one of - 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.

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  sensitive_params {
    secret_access_key            = # value needed
    secret_access_key_wo         = # value needed
    secret_access_key_wo_version = # value needed
  }
}


modular-magician avatar Jun 25 '25 03:06 modular-magician

Tests analytics

Total tests: 98 Passed tests: 91 Skipped tests: 5 Affected tests: 2

Click here to see the affected service packages
  • bigquerydatatransfer
  • monitoring
  • secretmanager
#### Action taken
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccMonitoringUptimeCheckConfig_update_wo
  • TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample

Get to know how VCR tests work

modular-magician avatar Jun 25 '25 03:06 modular-magician

🔴 Tests failed during RECORDING mode: TestAccMonitoringUptimeCheckConfig_update_wo [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician avatar Jun 25 '25 03:06 modular-magician

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

github-actions[bot] avatar Jun 27 '25 09:06 github-actions[bot]

Apologies for the delayed review; I've been caught up in a project and haven't had a chance to take the time to review this. I will be OOO next week but hope to go back through this the week after.

melinath avatar Jun 27 '25 23:06 melinath

(and the other related PR)

melinath avatar Jun 27 '25 23:06 melinath

Apologies for the delayed review; I've been caught up in a project and haven't had a chance to take the time to review this. I will be OOO next week but hope to go back through this the week after.

No worries!

FYI I will also be OOO the first two weeks of July, so I might be a bit slow in replies.

ramonvermeulen avatar Jun 28 '25 14:06 ramonvermeulen

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

github-actions[bot] avatar Jul 01 '25 09:07 github-actions[bot]

Both reviewers that have been on this change are unavailable for different periods. @melinath can you pick up review or hand off when you're back?

rileykarson avatar Jul 01 '25 17:07 rileykarson

yes, I can pick up the review now that I'm back.

melinath avatar Jul 07 '25 16:07 melinath

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

github-actions[bot] avatar Jul 08 '25 09:07 github-actions[bot]

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

github-actions[bot] avatar Jul 15 '25 09:07 github-actions[bot]

@modular-magician reassign-reviewer melinath

melinath avatar Jul 16 '25 17:07 melinath

Thanks for your patience! Overall this looks really great - definitely the right direction! But there are some issues we'll need to work out.

Thanks a lot for the extensive review! I just got back from OOO and it's quite busy. Will probably take some days to get back on all the points of feedback.

ramonvermeulen avatar Jul 17 '25 15:07 ramonvermeulen

@melinath @BBBmau

Tried to apply all of the feedback and also updated the connected PR (https://github.com/GoogleCloudPlatform/magic-modules/pull/14227) for expander behaviour.

Would love another review! Maybe it is also good to let the downstream generation run so that we can take a look and discuss the generated code as well. Eventually went for recursively creating the lineage within the method itself via buildCurrentPropLineage, at least for the existing write-only fields this seems to work well and SetDefault can just stay as it is and also run for all newly generated fields.

Will still do the test case for breaking changes when I have time, as discussed in: https://github.com/GoogleCloudPlatform/magic-modules/pull/14230/files#r2231552501.

EDIT:

Good news about the compatibility test, maybe someone else can follow the same steps to confirm it twice, but the int to string conversion seems to work without compatibility issues! I described all my testing steps in the following details block to make it reproducable:

OPEN ME

Here a brief walkthrough of the exact steps executed to do this test:

1. Terraform version

❯ terraform version
Terraform v1.12.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v6.45.0

2. Configuration

Masked out project id and numbers throughout the examples.

locals {
  project_id = "my-gcp-project"
}

provider "google" {
  project = local.project_id
}

resource "google_secret_manager_secret" "default" {
  secret_id = "test-secret-wo-compatibility"
  replication {
    auto {}
  }
}

resource "google_secret_manager_secret_version" "default" {
  secret                 = google_secret_manager_secret.default.id
  secret_data_wo_version = 1
  secret_data_wo         = "I am write-only ;)"
}

3. Applied configuration

❯ terraform apply -auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # google_secret_manager_secret.default will be created
  + resource "google_secret_manager_secret" "default" {
      + create_time           = (known after apply)
      + deletion_protection   = false
      + effective_annotations = (known after apply)
      + effective_labels      = {
          + "goog-terraform-provisioned" = "true"
        }
      + expire_time           = (known after apply)
      + id                    = (known after apply)
      + name                  = (known after apply)
      + project               = "my-gcp-project"
      + secret_id             = "test-secret-wo-compatibility"
      + terraform_labels      = {
          + "goog-terraform-provisioned" = "true"
        }

      + replication {
          + auto {
            }
        }
    }

  # google_secret_manager_secret_version.default will be created
  + resource "google_secret_manager_secret_version" "default" {
      + create_time            = (known after apply)
      + deletion_policy        = "DELETE"
      + destroy_time           = (known after apply)
      + enabled                = true
      + id                     = (known after apply)
      + is_secret_data_base64  = false
      + name                   = (known after apply)
      + secret                 = (known after apply)
      + secret_data_wo         = (write-only attribute)
      + secret_data_wo_version = 1
      + version                = (known after apply)
    }

Plan: 2 to add, 0 to change, 0 to destroy.
google_secret_manager_secret.default: Creating...
google_secret_manager_secret.default: Creation complete after 2s [id=projects/my-gcp-project/secrets/test-secret-wo-compatibility]
google_secret_manager_secret_version.default: Creating...
google_secret_manager_secret_version.default: Creation complete after 3s [id=projects/XXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1]

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

And the terraform state after apply looks like the following:

{
  "version": 4,
  "terraform_version": "1.12.2",
  "serial": 31,
  "lineage": "4812b715-adbd-0d16-f5f0-8be8ba3fe232",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "google_secret_manager_secret",
      "name": "default",
      "provider": "provider[\"registry.terraform.io/hashicorp/google\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "annotations": null,
            "create_time": "2025-07-30T06:18:08.589955Z",
            "deletion_protection": false,
            "effective_annotations": {},
            "effective_labels": {
              "goog-terraform-provisioned": "true"
            },
            "expire_time": "",
            "id": "projects/my-gcp-project/secrets/test-secret-wo-compatibility",
            "labels": null,
            "name": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility",
            "project": "my-gcp-project",
            "replication": [
              {
                "auto": [
                  {
                    "customer_managed_encryption": []
                  }
                ],
                "user_managed": []
              }
            ],
            "rotation": [],
            "secret_id": "test-secret-wo-compatibility",
            "tags": null,
            "terraform_labels": {
              "goog-terraform-provisioned": "true"
            },
            "timeouts": null,
            "topics": [],
            "ttl": null,
            "version_aliases": null,
            "version_destroy_ttl": ""
          },
          "sensitive_attributes": [],
          "identity_schema_version": 0,
          "private": "eyJlMmJmYjczMC1lY2FhLTExZTYtOGY4OC0zNDM2M2JjN2M0YzAiOnsiY3JlYXRlIjoxMjAwMDAwMDAwMDAwLCJkZWxldGUiOjEyMDAwMDAwMDAwMDAsInVwZGF0ZSI6MTIwMDAwMDAwMDAwMH19"
        }
      ]
    },
    {
      "mode": "managed",
      "type": "google_secret_manager_secret_version",
      "name": "default",
      "provider": "provider[\"registry.terraform.io/hashicorp/google\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "create_time": "2025-07-30T06:18:12.092229Z",
            "deletion_policy": "DELETE",
            "destroy_time": "",
            "enabled": true,
            "id": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1",
            "is_secret_data_base64": false,
            "name": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1",
            "secret": "projects/my-gcp-projectv/secrets/test-secret-wo-compatibility",
            "secret_data": null,
            "secret_data_wo": null,
            "secret_data_wo_version": 1,
            "timeouts": null,
            "version": "1"
          },
          "sensitive_attributes": [
            [
              {
                "type": "get_attr",
                "value": "secret_data"
              }
            ]
          ],
          "identity_schema_version": 0,
          "private": "eyJlMmJmYjczMC1lY2FhLTExZTYtOGY4OC0zNDM2M2JjN2M0YzAiOnsiY3JlYXRlIjoxMjAwMDAwMDAwMDAwLCJkZWxldGUiOjEyMDAwMDAwMDAwMDAsInVwZGF0ZSI6MTIwMDAwMDAwMDAwMH19",
          "dependencies": [
            "google_secret_manager_secret.default"
          ]
        }
      ]
    }
  ],
  "check_results": null
}

4. Build the provider based upon the branch of this PR

make provider VERSION=ga OUTPUT_PATH="$GOPATH/src/github.com/hashicorp/terraform-provider-google"
cd "$GOPATH/src/github.com/hashicorp/terraform-provider-google"
make build

And create the tf-dev-override.tfrc in the same directory as the terraform config:

provider_installation {
  dev_overrides {
      "hashicorp/google" = "/Users/ramon/go/bin/" # your bin location
      "hashicorp/google-beta" = "/Users/ramon/go/bin/"  # your bin location
  }
  direct {}
}

5. Execute terraform plan using TF_CLI_CONFIG_FILE environment variable

➜ TF_CLI_CONFIG_FILE="$(pwd)/tf-dev-override.tfrc" terraform plan

╷
│ Warning: Provider development overrides are in effect
│ 
│ The following provider development overrides are set in the CLI configuration:
│  - hashicorp/google in /Users/ramon/go/bin
│  - hashicorp/google-beta in /Users/ramon/go/bin
│ 
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
google_secret_manager_secret.default: Refreshing state... [id=projects/my-gcp-project/secrets/test-secret-wo-compatibility]
google_secret_manager_secret_version.default: Refreshing state... [id=projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

6. Execute terraform apply using TF_CLI_CONFIG_FILE environment variable

 
➜ TF_CLI_CONFIG_FILE="$(pwd)/tf-dev-override.tfrc" terraform apply 

╷
│ Warning: Provider development overrides are in effect
│ 
│ The following provider development overrides are set in the CLI configuration:
│  - hashicorp/google in /Users/ramon/go/bin
│  - hashicorp/google-beta in /Users/ramon/go/bin
│ 
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
google_secret_manager_secret.default: Refreshing state... [id=projects/my-gcp-project/secrets/test-secret-wo-compatibility]
google_secret_manager_secret_version.default: Refreshing state... [id=projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

7. Verify terraform state file afterwards, and indeed changed from int to string

{
  "version": 4,
  "terraform_version": "1.12.2",
  "serial": 32,
  "lineage": "4812b715-adbd-0d16-f5f0-8be8ba3fe232",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "google_secret_manager_secret",
      "name": "default",
      "provider": "provider[\"registry.terraform.io/hashicorp/google\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "annotations": {},
            "create_time": "2025-07-30T06:18:08.589955Z",
            "deletion_protection": false,
            "effective_annotations": {},
            "effective_labels": {
              "goog-terraform-provisioned": "true"
            },
            "expire_time": "",
            "id": "projects/my-gcp-project/secrets/test-secret-wo-compatibility",
            "labels": {},
            "name": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility",
            "project": "my-gcp-project",
            "replication": [
              {
                "auto": [
                  {
                    "customer_managed_encryption": []
                  }
                ],
                "user_managed": []
              }
            ],
            "rotation": [],
            "secret_id": "test-secret-wo-compatibility",
            "tags": null,
            "terraform_labels": {
              "goog-terraform-provisioned": "true"
            },
            "timeouts": null,
            "topics": [],
            "ttl": null,
            "version_aliases": {},
            "version_destroy_ttl": ""
          },
          "sensitive_attributes": [],
          "identity_schema_version": 0,
          "private": "eyJlMmJmYjczMC1lY2FhLTExZTYtOGY4OC0zNDM2M2JjN2M0YzAiOnsiY3JlYXRlIjoxMjAwMDAwMDAwMDAwLCJkZWxldGUiOjEyMDAwMDAwMDAwMDAsInVwZGF0ZSI6MTIwMDAwMDAwMDAwMH19"
        }
      ]
    },
    {
      "mode": "managed",
      "type": "google_secret_manager_secret_version",
      "name": "default",
      "provider": "provider[\"registry.terraform.io/hashicorp/google\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "create_time": "2025-07-30T06:18:12.092229Z",
            "deletion_policy": "DELETE",
            "destroy_time": "",
            "enabled": true,
            "id": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1",
            "is_secret_data_base64": false,
            "name": "projects/XXXXXXXXXXX/secrets/test-secret-wo-compatibility/versions/1",
            "secret": "projects/my-gcp-project/secrets/test-secret-wo-compatibility",
            "secret_data": null,
            "secret_data_wo": null,
            "secret_data_wo_version": "1",
            "timeouts": null,
            "version": "1"
          },
          "sensitive_attributes": [
            [
              {
                "type": "get_attr",
                "value": "secret_data"
              }
            ]
          ],
          "identity_schema_version": 0,
          "private": "eyJlMmJmYjczMC1lY2FhLTExZTYtOGY4OC0zNDM2M2JjN2M0YzAiOnsiY3JlYXRlIjoxMjAwMDAwMDAwMDAwLCJkZWxldGUiOjEyMDAwMDAwMDAwMDAsInVwZGF0ZSI6MTIwMDAwMDAwMDAwMH19",
          "dependencies": [
            "google_secret_manager_secret.default"
          ]
        }
      ]
    }
  ],
  "check_results": null
}

As you can see the "secret_data_wo_version": "1", changed from int to str.

I see the tests just ran, and the tests related to MonitoringUptimeCheck seem to fail, I will later check what is causing this and see if I can determine the problem and update the test cases if necessary.

ramonvermeulen avatar Jul 29 '25 19:07 ramonvermeulen

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 ( 8 files changed, 70 insertions(+), 69 deletions(-)) google-beta provider: Diff ( 8 files changed, 70 insertions(+), 69 deletions(-)) terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+), 11 deletions(-))

Breaking Change(s) Detected

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

  • Field http_check.auth_info.password_wo_version default value changed from <nil> to 0 on google_monitoring_uptime_check_config - reference
  • Field http_check.auth_info.password_wo within resource google_monitoring_uptime_check_config was added to exactly one of - reference
  • Field secret_data_wo_version changed from TypeInt to TypeString on google_secret_manager_secret_version - reference
  • Field sensitive_params.secret_access_key_wo_version changed from TypeInt to TypeString on google_bigquery_data_transfer_config - reference
  • Field sensitive_params.secret_access_key_wo_version default value changed from <nil> to 0 on google_bigquery_data_transfer_config - reference
  • Field sensitive_params.secret_access_key_wo within resource google_bigquery_data_transfer_config was added to exactly one of - reference
  • Field sensitive_params.secret_access_key within resource google_bigquery_data_transfer_config was added to exactly one of - 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.

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  sensitive_params {
    secret_access_key            = # value needed
    secret_access_key_wo         = # value needed
    secret_access_key_wo_version = # value needed
  }
}


modular-magician avatar Jul 29 '25 21:07 modular-magician

Tests analytics

Total tests: 101 Passed tests: 91 Skipped tests: 5 Affected tests: 5

Click here to see the affected service packages
  • monitoring
  • secretmanager
  • bigquerydatatransfer
#### 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
  • TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields
  • TestAccMonitoringUptimeCheckConfig_noProjectId
  • TestAccMonitoringUptimeCheckConfig_update
  • TestAccMonitoringUptimeCheckConfig_update_wo
  • TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample

Get to know how VCR tests work

modular-magician avatar Jul 29 '25 21:07 modular-magician

🔴 Tests failed during RECORDING mode: TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_noProjectId [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_update [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_update_wo [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician avatar Jul 29 '25 21:07 modular-magician

🔴 Tests failed during RECORDING mode: TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_noProjectId [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_update [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_update_wo [Error message] [Debug log] TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

These failing tests seems to be related to how the custom flatten is now generated for the _wo_version field, this should be the same as on the left side:

image

So just returning how it is configured, since it is a client-side only field.

Added a fix in https://github.com/GoogleCloudPlatform/magic-modules/pull/14230/commits/c147a5c5c164db841e4a8893165a7f51c755de2e.

All tests locally are succeeding now, except one but that seems to be a permission issue:

Logs
❯ make testacc TEST=./google/services/monitoring TESTARGS='-run=TestAccMonitoringUptimeCheckConfig_'

TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google/services/monitoring -v -run=TestAccMonitoringUptimeCheckConfig_ -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigStatusCodeExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigStatusCodeExample
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckTcpExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckTcpExample
=== RUN   TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigSyntheticMonitorExample
=== PAUSE TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigSyntheticMonitorExample
=== RUN   TestAccMonitoringUptimeCheckConfig_update
=== PAUSE TestAccMonitoringUptimeCheckConfig_update
=== RUN   TestAccMonitoringUptimeCheckConfig_update_wo
=== PAUSE TestAccMonitoringUptimeCheckConfig_update_wo
=== RUN   TestAccMonitoringUptimeCheckConfig_noProjectId
=== PAUSE TestAccMonitoringUptimeCheckConfig_noProjectId
=== RUN   TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields
=== PAUSE TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields
=== RUN   TestAccMonitoringUptimeCheckConfig_jsonPathUpdate
=== PAUSE TestAccMonitoringUptimeCheckConfig_jsonPathUpdate
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample
=== CONT  TestAccMonitoringUptimeCheckConfig_update
=== CONT  TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigSyntheticMonitorExample
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckTcpExample
=== CONT  TestAccMonitoringUptimeCheckConfig_noProjectId
=== CONT  TestAccMonitoringUptimeCheckConfig_jsonPathUpdate
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigStatusCodeExample
=== CONT  TestAccMonitoringUptimeCheckConfig_update_wo
=== CONT  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample
--- PASS: TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigStatusCodeExample (17.00s)
--- PASS: TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample (18.71s)
--- PASS: TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpPasswordWoExample (19.64s)
--- PASS: TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample (20.38s)
--- PASS: TestAccMonitoringUptimeCheckConfig_jsonPathUpdate (25.19s)
--- PASS: TestAccMonitoringUptimeCheckConfig_noProjectId (26.01s)
--- PASS: TestAccMonitoringUptimeCheckConfig_uptimeCheckTcpExample (27.30s)
--- PASS: TestAccMonitoringUptimeCheckConfig_update (28.24s)
--- PASS: TestAccMonitoringUptimeCheckConfig_update_wo (29.50s)
=== NAME  TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigSyntheticMonitorExample
    resource_monitoring_uptime_check_config_generated_test.go:372: Step 1/2 error: Error running apply: exit status 1

        Error: Error waiting to create function: Error waiting for Creating function: Error code 3, message: Build failed with status: FAILURE. Could not build the function due to a missing permission on the build service account. If you didn't revoke that permission explicitly, this could be caused by a change in the organization policies. Please refer to the following documentation for more details and resolution: https://cloud.google.com/functions/docs/troubleshooting#build-service-account
         You can also view the logs at https://console.cloud.google.com/cloud-build/builds;region=us-central1/ed61e683-c1bd-46a7-9f1c-d0525c195e8c?project=XXXXXXXX.


          with google_cloudfunctions2_function.function,
          on terraform_plugin_test.tf line 14, in resource "google_cloudfunctions2_function" "function":
          14: resource "google_cloudfunctions2_function" "function" {

--- FAIL: TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigSyntheticMonitorExample (34.39s)
--- PASS: TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields (37.70s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-google/google/services/monitoring       38.502s
FAIL
make: *** [testacc] Error 1

ramonvermeulen avatar Jul 31 '25 12:07 ramonvermeulen

Thanks! I'll take another pass on this tomorrow.

melinath avatar Jul 31 '25 23:07 melinath

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 ( 8 files changed, 68 insertions(+), 70 deletions(-)) google-beta provider: Diff ( 8 files changed, 68 insertions(+), 70 deletions(-)) terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+), 11 deletions(-))

Breaking Change(s) Detected

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

  • Field http_check.auth_info.password_wo within resource google_monitoring_uptime_check_config was added to exactly one of - reference
  • Field http_check.auth_info.password within resource google_monitoring_uptime_check_config was added to exactly one of - reference
  • Field secret_data_wo_version changed from TypeInt to TypeString on google_secret_manager_secret_version - reference
  • Field secret_data_wo_version default value changed from 0 to <nil> on google_secret_manager_secret_version - reference
  • Field sensitive_params.secret_access_key_wo_version changed from TypeInt to TypeString on google_bigquery_data_transfer_config - reference
  • Field sensitive_params.secret_access_key_wo within resource google_bigquery_data_transfer_config was added to exactly one of - reference
  • Field sensitive_params.secret_access_key within resource google_bigquery_data_transfer_config was added to exactly one of - 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.

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  sensitive_params {
    secret_access_key            = # value needed
    secret_access_key_wo         = # value needed
    secret_access_key_wo_version = # value needed
  }
}


modular-magician avatar Jul 31 '25 23:07 modular-magician

Tests analytics

Total tests: 61 Passed tests: 56 Skipped tests: 5 Affected tests: 0

Click here to see the affected service packages
  • bigquerydatatransfer
  • monitoring
  • secretmanager
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

modular-magician avatar Jul 31 '25 23:07 modular-magician

This is looking really great! I think the test failure is unrelated to your changes, but we can find out on the next test run.

Thanks again for reviewing! Will go over the comments this Sunday or Monday.

ramonvermeulen avatar Aug 01 '25 19:08 ramonvermeulen

removing from my queue

Thanks, applied all your feedback in https://github.com/GoogleCloudPlatform/magic-modules/pull/14230/commits/d8288aa5e1b7fed2b07c5d794347fc0c1c1f37d0 and https://github.com/GoogleCloudPlatform/magic-modules/pull/14230/commits/28e36ec354b7ceac38ec6397fa881235f581a3b9, I left two comments open because I wan't 100% sure if we should change this.

ramonvermeulen avatar Aug 04 '25 08:08 ramonvermeulen

responded to the questions :-)

Thanks!

Once this is merged I want to change these two towards the new wo generation, which I think is directly a good check to find out if the set-up works well for newly added write-only fields:

https://github.com/GoogleCloudPlatform/magic-modules/pull/14138 https://github.com/GoogleCloudPlatform/magic-modules/pull/13847

And after that I would like to work on https://github.com/hashicorp/terraform-provider-google/issues/22614 to make it easier to implement ephemeral resources based upon a data source.

ramonvermeulen avatar Aug 04 '25 16:08 ramonvermeulen

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 ( 8 files changed, 68 insertions(+), 69 deletions(-)) google-beta provider: Diff ( 8 files changed, 68 insertions(+), 69 deletions(-)) terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+), 11 deletions(-))

Breaking Change(s) Detected

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

  • Field http_check.auth_info.password_wo within resource google_monitoring_uptime_check_config was added to exactly one of - reference
  • Field http_check.auth_info.password within resource google_monitoring_uptime_check_config was added to exactly one of - reference
  • Field secret_data_wo_version changed from TypeInt to TypeString on google_secret_manager_secret_version - reference
  • Field secret_data_wo_version default value changed from 0 to <nil> on google_secret_manager_secret_version - reference
  • Field sensitive_params.secret_access_key_wo_version changed from TypeInt to TypeString on google_bigquery_data_transfer_config - reference
  • Field sensitive_params.secret_access_key_wo within resource google_bigquery_data_transfer_config was added to exactly one of - reference
  • Field sensitive_params.secret_access_key within resource google_bigquery_data_transfer_config was added to exactly one of - 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.

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  sensitive_params {
    secret_access_key            = # value needed
    secret_access_key_wo         = # value needed
    secret_access_key_wo_version = # value needed
  }
}


modular-magician avatar Aug 04 '25 16:08 modular-magician