-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added private origin authentication for cloud cdn 'compute_backend_service' #11042
Conversation
- Removed Required attribute from clientTlsPolicy and subjectAltNames fields;
…e effects of ignore_read on nested fields;
…yIgnore and added update test step; - Removed test for the iap.oauth2_client_secret field;
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. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_backend_service" "primary" {
security_settings {
client_tls_policy = # value needed
subject_alt_names = # value needed
}
}
|
Tests analyticsTotal tests: 965 Click here to see the affected service packages
Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
@BBBmau This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
…cdn-backend-service
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_backend_service" "primary" {
security_settings {
client_tls_policy = # value needed
subject_alt_names = # value needed
}
}
|
Tests analyticsTotal tests: 966 Click here to see the affected service packages
Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
…cdn-backend-service
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_backend_service" "primary" {
security_settings {
client_tls_policy = # value needed
subject_alt_names = # value needed
}
}
|
Tests analyticsTotal tests: 967 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
…cdn-backend-service
…endService; - Added test for clientTlsPolicy and subjectAltNames fields in BackendService;
@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. If no action is taken, this PR will be closed in 28 days. This notification can be disabled with the |
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 |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_backend_service" "primary" {
security_settings {
client_tls_policy = # value needed
subject_alt_names = # value needed
}
}
|
Tests analyticsTotal tests: 968 Click here to see the affected service packages
View the build log |
return fmt.Sprintf(` | ||
resource "google_compute_backend_service" "foobar" { | ||
name = "tf-test-backend-service-%s" | ||
security_settings { |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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;
…cdn-backend-service
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 973 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
There was a problem hiding this 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!
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)