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

feat(TPG >= 4.50)!: Adding edge_security_policy #311

Merged

Conversation

zsiegel
Copy link

@zsiegel zsiegel commented Mar 31, 2023

Adds the edge security policy option.

Note that when I ran make build, there were additional changes made that seem unrelated to mine, and I do not understand why. Happy to fix it if there are any pointers on what to adjust or run.

count                 = var.ssl || var.certificate_map != null ? 1 : 0

was changed back to

count                 = var.ssl ? 1 : 0

@zsiegel zsiegel requested review from a team and imrannayer as code owners March 31, 2023 20:54
@google-cla
Copy link

google-cla bot commented Mar 31, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@imrannayer
Copy link
Collaborator

imrannayer commented Mar 31, 2023

Check the code in template dir. Can you update the code in template directory and execute make build?

@imrannayer
Copy link
Collaborator

Can you also bump the provider version to 4.50 release which added this feature?

@apeabody
Copy link
Contributor

apeabody commented Mar 31, 2023

@imrannayer @bharathkkb - It appears the LINT actually failed for #305 "Error: The following tests have failed: check_generate_modules", however it reported back to GH as pass and so missed that an updated autogen/main.tf.tmpl wasn't included in that change. So the changes from #305 were (correctly per the template) reverted when make build was run for this PR.

@zsiegel
Copy link
Author

zsiegel commented Mar 31, 2023

Sure thing. I can fix the template. I suspected something was missed in a previous PR, but thanks for confirming. I'm on it.

@apeabody
Copy link
Contributor

Sure thing. I can fix the template. I suspected something was missed in a previous PR, but thanks for confirming. I'm on it.

Thanks @zsiegel!

@imrannayer
Copy link
Collaborator

/gcbrun

@apeabody apeabody changed the title feat: Adding edge_security_policy feat!(TPG >= 4.50): Adding edge_security_policy Mar 31, 2023
@apeabody apeabody changed the title feat!(TPG >= 4.50): Adding edge_security_policy feat(TPG >= 4.50)!: Adding edge_security_policy Mar 31, 2023
@imrannayer
Copy link
Collaborator

@zsiegel integration test is failing with following error

$$$$$$ Building the infrastructure based on the Terraform configuration...
       module.example.data.template_file.group-startup-script: Reading...
       module.example.data.template_file.group-startup-script: Read complete after 0s [id=a0411ec0218dbb9430d9a8828a0d50bb106ffa5388e2b1dfdb9b41e8c233c8f9]
       module.example.module.mig.data.google_compute_zones.available: Reading...
       module.example.module.mig.data.google_compute_zones.available: Read complete after 0s [id=projects/ci-int-lb-http-3107/regions/us-west1]
       
       Error: Invalid function argument
       
         on ../../../main.tf line 199, in resource "google_compute_backend_service" "default":
        199:   edge_security_policy = lookup(each.value, "edge_security_policy") == "" ? null : (lookup(each.value, "edge_security_policy") == null ? var.edge_security_policy : each.value.edge_security_policy)
           ├────────────────
           │ while calling lookup(inputMap, key, default...)
           │ each.value is object with 18 attributes
       
       Invalid value for "inputMap" parameter: the given object has no attribute
       "edge_security_policy".
>>>>>> Running the command `terraform apply -auto-approve -lock=true -lock-timeout=0s -input=false -no-color -parallelism=10 -refresh=true  ` failed due to a non-zero exit code of 1.
-----> Converging <multi-certs-default>...
$$$$$$ Reading the Terraform client version...
       Terraform v1.3.9
       on linux_amd64
       + provider registry.terraform.io/hashicorp/google v4.59.0
       + provider registry.terraform.io/hashicorp/google-beta v4.59.0
       + provider registry.terraform.io/hashicorp/random v3.4.3
       + provider registry.terraform.io/hashicorp/template v2.2.0
       + provider registry.terraform.io/hashicorp/tls v4.0.4
       
       Your version of Terraform is out of date! The latest version
       is 1.4.4. You can update by downloading from https://www.terraform.io/downloads.html
$$$$$$ Finished reading the Terraform client version.
$$$$$$ Verifying the Terraform client version is in the supported interval of >= 0.11.4, < 1.1.0...
$$$$$$ Verifying the Terraform client version failed. Set `driver.verify_version: true` to upgrade this warning to an error.
$$$$$$ Selecting the kitchen-terraform-multi-certs-default Terraform workspace...
$$$$$$ Finished selecting the kitchen-terraform-multi-certs-default Terraform workspace.
$$$$$$ Downloading the modules needed for the Terraform configuration...
       - example in ../../../examples/multiple-certs
       Downloading registry.terraform.io/terraform-google-modules/cloud-nat/google 2.2.2 for example.cloud-nat-group1...
       - example.cloud-nat-group1 in .terraform/modules/example.cloud-nat-group1
       Downloading registry.terraform.io/terraform-google-modules/cloud-nat/google 2.2.2 for example.cloud-nat-group2...
       - example.cloud-nat-group2 in .terraform/modules/example.cloud-nat-group2
       Downloading registry.terraform.io/terraform-google-modules/cloud-nat/google 2.2.2 for example.cloud-nat-group3...
       - example.cloud-nat-group3 in .terraform/modules/example.cloud-nat-group3
       - example.gce-lb-https in ../../..
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig1...
       - example.mig1 in .terraform/modules/example.mig1/modules/mig
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig1_template...
       - example.mig1_template in .terraform/modules/example.mig1_template/modules/instance_template
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig2...
       - example.mig2 in .terraform/modules/example.mig2/modules/mig
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig2_template...
       - example.mig2_template in .terraform/modules/example.mig2_template/modules/instance_template
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig3...
       - example.mig3 in .terraform/modules/example.mig3/modules/mig
       Downloading registry.terraform.io/terraform-google-modules/vm/google 7.9.0 for example.mig3_template...
       - example.mig3_template in .terraform/modules/example.mig3_template/modules/instance_template
$$$$$$ Finished downloading the modules needed for the Terraform configuration.
$$$$$$ Validating the Terraform configuration files...
       Success! The configuration is valid.
       
$$$$$$ Finished validating the Terraform configuration files.
$$$$$$ Building the infrastructure based on the Terraform configuration...
       module.example.data.template_file.group2-startup-script: Reading...
       module.example.data.template_file.group3-startup-script: Reading...
       module.example.data.template_file.group1-startup-script: Reading...
       module.example.data.template_file.group1-startup-script: Read complete after 0s [id=fdc54f37e51e231cf6dd8b82ac95567b1ab447ce92b6892104f0bee3165f14d9]
       module.example.data.template_file.group2-startup-script: Read complete after 0s [id=3722a1bfe9ff59699bb6a34720d84c6289959a74442ac59e7b757b2c3faf53ab]
       module.example.data.template_file.group3-startup-script: Read complete after 0s [id=c0aa152c441b3f202340deda0271fc71b605077eb530fb4b9fba2026d2282d54]
       module.example.module.mig2.data.google_compute_zones.available: Reading...
       module.example.module.mig1.data.google_compute_zones.available: Reading...
       module.example.module.mig3.data.google_compute_zones.available: Reading...
       module.example.module.mig3.data.google_compute_zones.available: Read complete after 0s [id=projects/ci-int-lb-http-3107/regions/us-east1]
       module.example.module.mig1.data.google_compute_zones.available: Read complete after 0s [id=projects/ci-int-lb-http-3107/regions/us-west1]
       module.example.module.mig2.data.google_compute_zones.available: Read complete after 0s [id=projects/ci-int-lb-http-3107/regions/us-central1]
       
       Error: Invalid function argument
       
         on ../../../main.tf line 199, in resource "google_compute_backend_service" "default":
        199:   edge_security_policy = lookup(each.value, "edge_security_policy") == "" ? null : (lookup(each.value, "edge_security_policy") == null ? var.edge_security_policy : each.value.edge_security_policy)
           ├────────────────
           │ while calling lookup(inputMap, key, default...)
           │ each.value is object with 18 attributes
       
       Invalid value for "inputMap" parameter: the given object has no attribute
       "edge_security_policy".
       
       Error: Invalid function argument
       
         on ../../../main.tf line 199, in resource "google_compute_backend_service" "default":
        199:   edge_security_policy = lookup(each.value, "edge_security_policy") == "" ? null : (lookup(each.value, "edge_security_policy") == null ? var.edge_security_policy : each.value.edge_security_policy)
           ├────────────────
           │ while calling lookup(inputMap, key, default...)
           │ each.value is object with 18 attributes
       
       Invalid value for "inputMap" parameter: the given object has no attribute
       "edge_security_policy".
       
       Error: Invalid function argument
       
         on ../../../main.tf line 199, in resource "google_compute_backend_service" "default":
        199:   edge_security_policy = lookup(each.value, "edge_security_policy") == "" ? null : (lookup(each.value, "edge_security_policy") == null ? var.edge_security_policy : each.value.edge_security_policy)
           ├────────────────
           │ while calling lookup(inputMap, key, default...)
           │ each.value is object with 18 attributes
       
       Invalid value for "inputMap" parameter: the given object has no attribute
       "edge_security_policy".
       
       Error: Invalid function argument
       
         on ../../../main.tf line 199, in resource "google_compute_backend_service" "default":
        199:   edge_security_policy = lookup(each.value, "edge_security_policy") == "" ? null : (lookup(each.value, "edge_security_policy") == null ? var.edge_security_policy : each.value.edge_security_policy)
           ├────────────────
           │ while calling lookup(inputMap, key, default...)
           │ each.value is object with 18 attributes
       
       Invalid value for "inputMap" parameter: the given object has no attribute
       "edge_security_policy".

@zsiegel
Copy link
Author

zsiegel commented Apr 3, 2023

Thanks for this! I will take a look this week and see if I can get it all cleared up.

@zsiegel
Copy link
Author

zsiegel commented Apr 4, 2023

Ok I think I missed a ref for the backends in the autogen config. Let's give this a try!

I was having some issues getting my local setup and I'm traveling but if this does not work I will need to get my local testing setup properly.

Thanks

@zsiegel
Copy link
Author

zsiegel commented Apr 5, 2023

Ok I think I missed a ref for the backends in the autogen config. Let's give this a try!

I was having some issues getting my local setup and I'm traveling but if this does not work I will need to get my local testing setup properly.

Thanks

I actually suspect I need to update the metadata.yml file as well but that is a little unclear to me at this time. Let me know if that's appropriate.

@zsiegel
Copy link
Author

zsiegel commented Apr 5, 2023

Ok I think I missed a ref for the backends in the autogen config. Let's give this a try!
I was having some issues getting my local setup and I'm traveling but if this does not work I will need to get my local testing setup properly.
Thanks

I actually suspect I need to update the metadata.yml file as well but that is a little unclear to me at this time. Let me know if that's appropriate.

I updated this :)

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

@zsiegel Lint check failed and also integration test is throwing following errors:

       Error: Invalid value for input variable
       
         on ../../../examples/mig-nat-http-lb/main.tf line 101, in module "gce-lb-http":
        101:   backends = {
        102:     default = {
        103:       description                     = null
        104:       protocol                        = "HTTP"
        105:       port                            = 80
        106:       port_name                       = "http"
        107:       timeout_sec                     = 10
        108:       connection_draining_timeout_sec = null
        109:       enable_cdn                      = false
        110:       compression_mode                = null
        111:       security_policy                 = null
        112:       session_affinity                = null
        113:       affinity_cookie_ttl_sec         = null
        114:       custom_request_headers          = null
        115:       custom_response_headers         = null
        116:       health_check = {
        117:         check_interval_sec  = null
        118:         timeout_sec         = null
        119:         healthy_threshold   = null
        120:         unhealthy_threshold = null
        121:         request_path        = "/"
        122:         port                = 80
        123:         host                = null
        124:         logging             = null
        125:       }
        126:       log_config = {
        127:         enable      = false
        128:         sample_rate = null
        129:       }
        130:       groups = [
        131:         {
        132:           group                        = module.mig.instance_group
        133:           balancing_mode               = null
        134:           capacity_scaler              = null
        135:           description                  = null
        136:           max_connections              = null
        137:           max_connections_per_instance = null
        138:           max_connections_per_endpoint = null
        139:           max_rate                     = null
        140:           max_rate_per_instance        = null
        141:           max_rate_per_endpoint        = null
        142:           max_utilization              = null
        143:         }
        144:       ]
        145:       iap_config = {
        146:         enable               = false
        147:         oauth2_client_id     = ""
        148:         oauth2_client_secret = ""
        149:       }
        150:     }
        151:   }
       
       The given value is not suitable for
       module.example.module.gce-lb-http.var.backends declared at
       ../../../variables.tf:81,1-20: element "default": attribute
       "edge_security_policy" is required.

       Error: Invalid value for input variable
       
         on ../../../examples/multiple-certs/main.tf line 139, in module "gce-lb-https":
        139:   backends = {
        140:     default = {
        141:       description                     = null
        142:       protocol                        = "HTTP"
        143:       port                            = 80
        144:       port_name                       = "http"
        145:       timeout_sec                     = 10
        146:       connection_draining_timeout_sec = null
        147:       enable_cdn                      = false
        148:       compression_mode                = null
        149:       security_policy                 = null
        150:       session_affinity                = null
        151:       affinity_cookie_ttl_sec         = null
        152:       custom_request_headers          = null
        153:       custom_response_headers         = null
        154:       health_check = local.health_check
        155:       log_config = {
        156:         enable      = true
        157:         sample_rate = 1.0
        158:       }
        159:       groups = [
        160:         {
        161:           group                        = module.mig1.instance_group
        162:           balancing_mode               = null
        163:           capacity_scaler              = null
        164:           description                  = null
        165:           max_connections              = null
        166:           max_connections_per_instance = null
        167:           max_connections_per_endpoint = null
        168:           max_rate                     = null
        169:           max_rate_per_instance        = null
        170:           max_rate_per_endpoint        = null
        171:           max_utilization              = null
        172:         },
        173:         {
        174:           group                        = module.mig2.instance_group
        175:           balancing_mode               = null
        176:           capacity_scaler              = null
        177:           description                  = null
        178:           max_connections              = null
        179:           max_connections_per_instance = null
        180:           max_connections_per_endpoint = null
        181:           max_rate                     = null
        182:           max_rate_per_instance        = null
        183:           max_rate_per_endpoint        = null
        184:           max_utilization              = null
        185:         },
        186:         {
        187:           group                        = module.mig3.instance_group
        188:           balancing_mode               = null
        189:           capacity_scaler              = null
        190:           description                  = null
        191:           max_connections              = null
        192:           max_connections_per_instance = null
        193:           max_connections_per_endpoint = null
        194:           max_rate                     = null
        195:           max_rate_per_instance        = null
        196:           max_rate_per_endpoint        = null
        197:           max_utilization              = null
        198:         },
        199:       ]
        200:       iap_config = {
        201:         enable               = false
        202:         oauth2_client_id     = ""
        203:         oauth2_client_secret = ""
        204:       }
        205:     }
        206:     mig1 = {
        207:       description                     = null
        208:       protocol                        = "HTTP"
        209:       port                            = 80
        210:       port_name                       = "http"
        211:       timeout_sec                     = 10
        212:       connection_draining_timeout_sec = null
        213:       enable_cdn                      = false
        214:       compression_mode                = null
        215:       security_policy                 = null
        216:       session_affinity                = null
        217:       affinity_cookie_ttl_sec         = null
        218:       custom_request_headers          = null
        219:       custom_response_headers         = null
        220:       health_check = local.health_check
        221:       log_config = {
        222:         enable      = true
        223:         sample_rate = 1.0
        224:       }
        225:       groups = [
        226:         {
        227:           group                        = module.mig1.instance_group
        228:           balancing_mode               = null
        229:           capacity_scaler              = null
        230:           description                  = null
        231:           max_connections              = null
        232:           max_connections_per_instance = null
        233:           max_connections_per_endpoint = null
        234:           max_rate                     = null
        235:           max_rate_per_instance        = null
        236:           max_rate_per_endpoint        = null
        237:           max_utilization              = null
        238:         },
        239:       ]
        240:       iap_config = {
        241:         enable               = false
        242:         oauth2_client_id     = ""
        243:         oauth2_client_secret = ""
        244:       }
        245:     }
        246:     mig2 = {
        247:       description                     = null
        248:       protocol                        = "HTTP"
        249:       port                            = 80
        250:       port_name                       = "http"
        251:       timeout_sec                     = 10
        252:       connection_draining_timeout_sec = null
        253:       enable_cdn                      = false
        254:       compression_mode                = null
        255:       security_policy                 = null
        256:       session_affinity                = null
        257:       affinity_cookie_ttl_sec         = null
        258:       custom_request_headers          = null
        259:       custom_response_headers         = null
        260:       health_check = local.health_check
        261:       log_config = {
        262:         enable      = true
        263:         sample_rate = 1.0
        264:       }
        265:       groups = [
        266:         {
        267:           group                        = module.mig2.instance_group
        268:           balancing_mode               = null
        269:           capacity_scaler              = null
        270:           description                  = null
        271:           max_connections              = null
        272:           max_connections_per_instance = null
        273:           max_connections_per_endpoint = null
        274:           max_rate                     = null
        275:           max_rate_per_instance        = null
        276:           max_rate_per_endpoint        = null
        277:           max_utilization              = null
        278:         },
        279:       ]
        280:       iap_config = {
        281:         enable               = false
        282:         oauth2_client_id     = ""
        283:         oauth2_client_secret = ""
        284:       }
        285:     }
        286:     mig3 = {
        287:       description                     = null
        288:       protocol                        = "HTTP"
        289:       port                            = 80
        290:       port_name                       = "http"
        291:       timeout_sec                     = 10
        292:       connection_draining_timeout_sec = null
        293:       enable_cdn                      = false
        294:       compression_mode                = null
        295:       security_policy                 = null
        296:       session_affinity                = null
        297:       affinity_cookie_ttl_sec         = null
        298:       custom_request_headers          = null
        299:       custom_response_headers         = null
        300:       health_check = local.health_check
        301:       log_config = {
        302:         enable      = true
        303:         sample_rate = 1.0
        304:       }
        305:       groups = [
        306:         {
        307:           group                        = module.mig3.instance_group
        308:           balancing_mode               = null
        309:           capacity_scaler              = null
        310:           description                  = null
        311:           max_connections              = null
        312:           max_connections_per_instance = null
        313:           max_connections_per_endpoint = null
        314:           max_rate                     = null
        315:           max_rate_per_instance        = null
        316:           max_rate_per_endpoint        = null
        317:           max_utilization              = null
        318:         },
        319:       ]
        320:       iap_config = {
        321:         enable               = false
        322:         oauth2_client_id     = ""
        323:         oauth2_client_secret = ""
        324:       }
        325:     }
        326:   }
       
       The given value is not suitable for
       module.example.module.gce-lb-https.var.backends declared at
       ../../../variables.tf:81,1-20: element "default": attribute
       "edge_security_policy" is required.

@zsiegel
Copy link
Author

zsiegel commented Apr 6, 2023

@imrannayer I fixed the lint issues.

I am set up to run the tests via my local machine now, but I am running into quota issues. I submitted a request to see if I can get those increased!

@zsiegel
Copy link
Author

zsiegel commented Apr 7, 2023

@imrannayer I ran the tests locally and everything passed except for one thing.

Profile: https_redirect
Version: (not specified)
Target:  local://

  ×  https: HTTPs Check (2 failed)
     ×  HTTP GET on http://34.98.108.36 status 
     end of file reached
     ×  HTTP GET on https://34.98.108.36 status 
     SSL_connect SYSCALL returned=5 errno=0 state=SSLv3/TLS write client hello


Profile Summary: 0 successful controls, 1 control failure, 0 controls skipped
Test Summary: 0 successful, 2 failures, 0 skipped
>>>>>> Verifying the 'https_redirect' system failed:
	Running InSpec failed:
		Running InSpec failed due to a non-zero exit code of 1.
>>>>>> ------Exception-------
>>>>>> Class: Kitchen::ActionFailed
>>>>>> Message: 1 actions failed.
>>>>>>     Verify failed on instance <https-redirect-default>.  Please see .kitchen/logs/https-redirect-default.log for more details
>>>>>> ----------------------
>>>>>> Please see .kitchen/logs/kitchen.log for more details
>>>>>> Also try running `kitchen diagnose --all` for configuration

This looks like a random/transient error to me?

@imrannayer
Copy link
Collaborator

Do t worry about that error. Update ur PR.

@imrannayer
Copy link
Collaborator

imrannayer commented Apr 7, 2023

@zsiegel can you plz execute Make build and also generate doc using docker_generate_docs? Some changes for certificate manager are missing from main.tf.
Since this is breaking change can u plz create a guide for upgrade to v9.0. Use this as an example.

@imrannayer
Copy link
Collaborator

@zsiegel can you plz execute Make build. Main.tf is still missing certificate manager change.

@zsiegel
Copy link
Author

zsiegel commented Apr 7, 2023

@zsiegel can you plz execute Make build. Main.tf is still missing certificate manager change.

I did this already but let me give it another try.

@zsiegel
Copy link
Author

zsiegel commented Apr 7, 2023

@zsiegel can you plz execute Make build. Main.tf is still missing certificate manager change.

I did this already but let me give it another try.

I ran this again and am not seeing any changes. Are you referring to the certificate manager changes for the certificate_map? I can investigate further.

@imrannayer
Copy link
Collaborator

imrannayer commented Apr 10, 2023

@zsiegel leme know when you can make the suggested change.

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

LGTM

@bharathkkb bharathkkb merged commit f769bf7 into terraform-google-modules:master Apr 11, 2023
@imrannayer
Copy link
Collaborator

@zsiegel Thanks for the PR

@zsiegel zsiegel deleted the feat-edge-security-policy branch April 11, 2023 23:42
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