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

Added private origin authentication for cloud cdn 'compute_backend_service' #11042

Conversation

matheusaleixo-cit
Copy link
Contributor

@matheusaleixo-cit matheusaleixo-cit commented Jun 25, 2024

Adds support to 'aws_v4_authentication' and it's nested fields to 'compute_backend_service' resource.
Also updates the 'clientTlsPolicy' field of 'compute_backend_service' so it is set to reference the correct resource, and added relevant tests.

Fixes: hashicorp/terraform-provider-google#18551

Release Note Template for Downstream PRs (will be copied)

compute: added 'aws_v4_authentication' fields to 'google_compute_backend_service' resource;

- Removed Required attribute from clientTlsPolicy and subjectAltNames fields;
@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jun 25, 2024
…yIgnore and added update test step;

- Removed test for the iap.oauth2_client_secret field;
@matheusaleixo-cit matheusaleixo-cit marked this pull request as ready for review June 26, 2024 12:56
Copy link

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

@BBBmau, 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 BBBmau June 26, 2024 12:57
@modular-magician modular-magician added service/compute-l7-load-balancer and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Jun 27, 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 ( 4 files changed, 267 insertions(+), 13 deletions(-))
google-beta provider: Diff ( 4 files changed, 270 insertions(+), 16 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 63 insertions(+))

Missing test report

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

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

resource "google_compute_backend_service" "primary" {
  security_settings {
    client_tls_policy = # value needed
    subject_alt_names = # value needed
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 965
Passed tests: 890
Skipped tests: 73
Affected tests: 2

Click here to see the affected service packages
  • compute

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
  • TestAccComputeBackendService_withPrivateOriginAuth
  • TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[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 Jul 1, 2024

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

@BBBmau
Copy link
Collaborator

BBBmau commented Jul 2, 2024

TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy: This test was most likely fixed in a recent commit. Can you rebase?

@github-actions github-actions bot requested a review from BBBmau July 2, 2024 13:47
@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 2, 2024
@modular-magician modular-magician removed the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 3, 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 ( 4 files changed, 267 insertions(+), 13 deletions(-))
google-beta provider: Diff ( 4 files changed, 270 insertions(+), 16 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 63 insertions(+))

Missing test report

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

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

resource "google_compute_backend_service" "primary" {
  security_settings {
    client_tls_policy = # value needed
    subject_alt_names = # value needed
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 966
Passed tests: 890
Skipped tests: 73
Affected tests: 3

Click here to see the affected service packages
  • compute

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy
  • TestAccComputeNetworkFirewallPolicyRule_multipleRules
  • TestAccComputeNetworkFirewallPolicyRule_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]
TestAccComputeNetworkFirewallPolicyRule_multipleRules[Error message] [Debug log]
TestAccComputeNetworkFirewallPolicyRule_update[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

@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 Jul 10, 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 ( 4 files changed, 267 insertions(+), 13 deletions(-))
google-beta provider: Diff ( 4 files changed, 270 insertions(+), 16 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 63 insertions(+))

Missing test report

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

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

resource "google_compute_backend_service" "primary" {
  security_settings {
    client_tls_policy = # value needed
    subject_alt_names = # value needed
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 967
Passed tests: 893
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages
  • compute

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
  • TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[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

@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 12, 2024
…endService;

- Added test for clientTlsPolicy and subjectAltNames fields in BackendService;
Copy link

@matheusaleixo-cit, 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.

@modular-magician modular-magician removed the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 16, 2024
@BBBmau
Copy link
Collaborator

BBBmau commented Jul 16, 2024

All though these tests pass you are still missing two fields in the tests. This is seen from the recent VCR report here: #11042 (comment)

If you can include a test that also includes these two fields we should be able to get this approved and merged @matheusaleixo-cit

@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 ( 4 files changed, 267 insertions(+), 13 deletions(-))
google-beta provider: Diff ( 4 files changed, 270 insertions(+), 16 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 63 insertions(+))

Missing test report

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

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

resource "google_compute_backend_service" "primary" {
  security_settings {
    client_tls_policy = # value needed
    subject_alt_names = # value needed
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 968
Passed tests: 894
Skipped tests: 74
Affected tests: 0

Click here to see the affected service packages
  • compute

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

View the build log

return fmt.Sprintf(`
resource "google_compute_backend_service" "foobar" {
name = "tf-test-backend-service-%s"
security_settings {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Although tests are missing you are still missing two fields that must be included somewhere in the tests. This is seen by the recent VCR report: #11042 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, I needed to fix some issues with the current implementation of the 'clientTlsPolicy' field. I created a new test for these fields since they cannot be used together with the 'aws_v4_authentication' fields.

…ficate to the ClientTlsPolicy resource used and added a update step;

- Added diff_suppress_func to clientTlsPolicy field in BackendService;
@github-actions github-actions bot requested a review from BBBmau July 19, 2024 14:56
@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 19, 2024
@modular-magician modular-magician removed the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jul 19, 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 ( 4 files changed, 381 insertions(+), 18 deletions(-))
google-beta provider: Diff ( 4 files changed, 384 insertions(+), 21 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 63 insertions(+), 4 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 973
Passed tests: 898
Skipped tests: 74
Affected tests: 1

Click here to see the affected service packages
  • compute

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
  • TestAccComputeBackendService_withClientTlsPolicy

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeBackendService_withClientTlsPolicy[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
Collaborator

@BBBmau BBBmau left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

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 Private origin authentication support for Cloud CDN
3 participants