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 support for PrivateRegistryAccessConfig #10450

Closed
wants to merge 1 commit into from

Conversation

mmiranda96
Copy link
Contributor

This PR adds support for PrivateRegistryAccessConfig. See the public docs:

Release Note Template for Downstream PRs (will be copied)

container: added `containerd_config`, `private_registry_access_config`, `certificate_authority_domain_config`, and  `gcp_secret_manager_certificate_config` fields to `google_container_node_config`

Copy link

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

@shuyama1, 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 github-actions bot requested a review from shuyama1 April 15, 2024 17:44
@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Apr 15, 2024
@mmiranda96
Copy link
Contributor Author

PR is pending of a new version of https://github.com/googleapis/google-api-go-client being cut that includes googleapis/google-api-go-client#2516. Once this is available I will update go.mod and fix dependencies.

@modular-magician modular-magician added service/container and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Apr 16, 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 ( 5 files changed, 483 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 5 files changed, 484 insertions(+), 2 deletions(-))

Errors

google provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

google-beta provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

@mmiranda96
Copy link
Contributor Author

New googleapis/google-api-go-client version has been cut and upgraded to in this PR, it's ready for review

@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 Apr 17, 2024
@shuyama1
Copy link
Member

New googleapis/google-api-go-client version has been cut and upgraded to in this PR, it's ready for review

Thanks for the update. I just triggered the tests. Plus, you can follow the steps at go/terraform-contribution-guide#before-you-begin to join the org membership to get the tests automatically run for your PRs

@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 ( 7 files changed, 486 insertions(+), 4 deletions(-))
google-beta provider: Diff ( 7 files changed, 487 insertions(+), 5 deletions(-))

Missing test report

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

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

resource "google_container_cluster" "primary" {
  node_config {
    containerd_config = # value needed
    containerd_config {
      private_registry_access_config = # value needed
      private_registry_access_config {
        certificate_authority_domain_config = # value needed
        certificate_authority_domain_config {
          fqdns                                 = # value needed
          gcp_secret_manager_certificate_config = # value needed
          gcp_secret_manager_certificate_config {
            secret_uri = # value needed
          }
        }
        enabled = # value needed
      }
    }
  }
  node_pool {
    node_config {
      containerd_config = # value needed
      containerd_config {
        private_registry_access_config = # value needed
        private_registry_access_config {
          certificate_authority_domain_config = # value needed
          certificate_authority_domain_config {
            fqdns                                 = # value needed
            gcp_secret_manager_certificate_config = # value needed
            gcp_secret_manager_certificate_config {
              secret_uri = # value needed
            }
          }
          enabled = # value needed
        }
      }
    }
  }
  node_pool_defaults {
    node_config_defaults {
      containerd_config = # value needed
      containerd_config {
        private_registry_access_config = # value needed
        private_registry_access_config {
          certificate_authority_domain_config = # value needed
          certificate_authority_domain_config {
            gcp_secret_manager_certificate_config = # value needed
          }
        }
      }
    }
  }
}

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

resource "google_container_node_pool" "primary" {
  node_config {
    containerd_config = # value needed
    containerd_config {
      private_registry_access_config = # value needed
      private_registry_access_config {
        certificate_authority_domain_config = # value needed
        certificate_authority_domain_config {
          gcp_secret_manager_certificate_config = # value needed
        }
      }
    }
  }
}

@shuyama1
Copy link
Member

Unit tests failed with error

go: updates to go.mod needed; to update it:
	go mod tidy
make: *** [Makefile:10: test] Error 1

you may need to run go mod tidy against the generated local provider and copy the change in go.mod and go.sum back to the files in Magic Modules.

@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 Apr 18, 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 ( 7 files changed, 488 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 7 files changed, 489 insertions(+), 7 deletions(-))

Missing test report

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

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

resource "google_container_cluster" "primary" {
  node_config {
    containerd_config {
      private_registry_access_config {
        certificate_authority_domain_config {
          fqdns = # value needed
          gcp_secret_manager_certificate_config {
            secret_uri = # value needed
          }
        }
        enabled = # value needed
      }
    }
  }
  node_pool {
    node_config {
      containerd_config {
        private_registry_access_config {
          certificate_authority_domain_config {
            fqdns = # value needed
            gcp_secret_manager_certificate_config {
              secret_uri = # value needed
            }
          }
          enabled = # value needed
        }
      }
    }
  }
}

Copy link
Member

@shuyama1 shuyama1 left a comment

Choose a reason for hiding this comment

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

Looks like there're still issues with dependencies. Would you mind following the steps in yaqs/8444818848342867968#a1n2 to update the go.mod and go.sum files one more time. Also maybe make sure that your local magic-modules and provider repos are at the latest version(commit). Thanks a lot.

Tests + checks failed with error:

go: google.golang.org/genproto/googleapis/rpc@v0.0.0-20240314234333-6e1732d8331c: missing go.sum entry for go.mod file; to add it:
	go mod download google.golang.org/genproto/googleapis/rpc
make: *** [GNUmakefile:2[9](https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8745815227/job/24001533550?pr=10450#step:9:10): vet] Error 1

@mmiranda96
Copy link
Contributor Author

@vivzbansal will take ownership of this work. Thanks Vivek!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants