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

Fix handling of SSL certificates in external load balancer modules #2780

Conversation

rodriguezsergio
Copy link
Contributor

@rodriguezsergio rodriguezsergio commented Dec 20, 2024

An empty ssl_certificates list will conflict with a user-defined certificate_manager_certificates value, so exclude it.

This was fixed in #2764, but only for the net-lb-app-int module and not for the net-lb-app-ext or net-lb-app-ext-regional modules.

I'm not sure why net-lb-app-int-cross-region doesn't allow a user to set a value for ssl_certificates, but I'm going to leave it alone.

Screenshot from #2760


Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@rodriguezsergio
Copy link
Contributor Author

Test output

Running tests for net-lb-app-ext-regional

================================================================================= test session starts =================================================================================
platform darwin -- Python 3.12.5, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/srodriguez/code/github/googlecloudplatform/cloud-foundation-fabric
plugins: xdist-3.6.1
collected 1015 items / 1000 deselected / 15 selected

tests/examples/test_plan.py ...............                                                                                                                                     [100%]

=================================================================== 15 passed, 1000 deselected in 275.37s (0:04:35) ===================================================================
Running tests for net-lb-app-ext

================================================================================= test session starts =================================================================================
platform darwin -- Python 3.12.5, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/srodriguez/code/github/googlecloudplatform/cloud-foundation-fabric
plugins: xdist-3.6.1
collected 1015 items / 994 deselected / 21 selected

tests/examples/test_plan.py .....................                                                                                                                               [100%]

=================================================================== 21 passed, 994 deselected in 460.19s (0:07:40) ====================================================================
Running tests for net-lb-app-int

================================================================================= test session starts =================================================================================
platform darwin -- Python 3.12.5, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/srodriguez/code/github/googlecloudplatform/cloud-foundation-fabric
plugins: xdist-3.6.1
collected 1015 items / 999 deselected / 16 selected

tests/examples/test_plan.py ................                                                                                                                                    [100%]

=================================================================== 16 passed, 999 deselected in 557.24s (0:09:17) ====================================================================

An empty `ssl_certificates` list will conflict with a user-defined
`certificate_manager_certificates` value, so exclude it.
@rodriguezsergio rodriguezsergio force-pushed the rodriguezsergio/mutually_exclusive_certs branch from fae478a to bf9176f Compare December 20, 2024 20:41
@rodriguezsergio rodriguezsergio changed the title an empty ssl_certificates list should be set to null remove conflict potential between two *_certificates attributes Dec 20, 2024
@ludoo ludoo enabled auto-merge (squash) December 21, 2024 10:25
@ludoo ludoo changed the title remove conflict potential between two *_certificates attributes Fix handling of SSL certificates in external load balancer modules Dec 21, 2024
@ludoo ludoo merged commit 1e4a3a4 into GoogleCloudPlatform:master Dec 21, 2024
14 checks passed
@ludoo
Copy link
Collaborator

ludoo commented Dec 21, 2024

Thanks for this, and the detailed explanation!

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.

2 participants