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 security group support to network load balancer #2079

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

erinkdelong
Copy link

@erinkdelong erinkdelong commented Apr 15, 2024

SUMMARY

Add security groups to Network Load Balancers for AWS

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

elb_network_lb

ADDITIONAL INFORMATION

In the past, NLBs did not accept security groups. Now you can create NLBs with security groups. However, if you don't add a security group when you create the NLB, you cannot add one later. That means if you ever want security groups on your NLB, you cannot use the existing module.

I created this PR by subclassing the exiting NetworkLoadBalancer class in the aws.amazon project, and added the security group support as seen in that project.

I was not able to get the integration tests to run. I tried on multiple computers, using a clean fork, but it always failed. (https://stackoverflow.com/questions/78289663/error-running-ansible-test-on-community-aws-collection). So I'm unsure that these changes actually work. I did add integration tests, however.

This is my first attempt at contributing to Ansible. I'm not sure that this is the correct way to do this, and in any event these changes need to be ported over to the aws.amazon project.

Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/ansible-collections/community.aws for 2079,b2a2570b82936f04c7df7176c13f1f3ebb0fba5e

Copy link

github-actions bot commented Apr 15, 2024

Docs Build 📝

Thank you for contribution!✨

The docsite for this PR is available for download as an artifact from this run:
https://github.com/ansible-collections/community.aws/actions/runs/9021253156

You can compare to the docs for the main branch here:
https://ansible-collections.github.io/community.aws/branch/main

File changes:

  • M collections/community/aws/ecs_cluster_module.html
  • M collections/community/aws/ecs_service_module.html
  • M collections/community/aws/elb_network_lb_module.html
  • M collections/community/aws/glue_connection_module.html
Click to see the diff comparison.

NOTE: only file modifications are shown here. New and deleted files are excluded.
See the file list and check the published docs to see those files.

diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_cluster_module.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_cluster_module.html
index 6b82271..c69854f 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_cluster_module.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_cluster_module.html
@@ -311,7 +311,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
 <p><em class="ansible-option-versionadded">added in community.aws 5.2.0</em></p>
 </div></td>
 <td><div class="ansible-option-cell"><p>Toggle overwriting of existing capacity providers or strategy. This is needed for backwards compatibility.</p>
-<p>By default <em>purge_capacity_providers=false</em>.  In a release after 2024-06-01 this will be changed to <em>purge_capacity_providers=true</em>.</p>
+<p>By default <em>purge_capacity_providers=false</em>.  In release 9.0.0 this default will be changed to <em>purge_capacity_providers=true</em>.</p>
 <p class="ansible-option-line"><strong class="ansible-option-choices">Choices:</strong></p>
 <ul class="simple">
 <li><p><code class="ansible-option-default-bold docutils literal notranslate"><strong><span class="pre">false</span></strong></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_service_module.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_service_module.html
index 60dc4cb..8ce1e05 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_service_module.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_service_module.html
@@ -575,7 +575,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-s
 <p><em class="ansible-option-versionadded">added in community.aws 5.3.0</em></p>
 </div></td>
 <td><div class="ansible-option-cell"><p>Toggle overwriting of existing placement constraints. This is needed for backwards compatibility.</p>
-<p>By default <em>purge_placement_constraints=false</em>. In a release after 2024-06-01 this will be changed to <em>purge_placement_constraints=true</em>.</p>
+<p>By default <em>purge_placement_constraints=false</em>. In release 9.0.0 this will be changed to <em>purge_placement_constraints=true</em>.</p>
 <p class="ansible-option-line"><strong class="ansible-option-choices">Choices:</strong></p>
 <ul class="simple">
 <li><p><code class="ansible-option-default-bold docutils literal notranslate"><strong><span class="pre">false</span></strong></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
@@ -589,7 +589,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-s
 <p><em class="ansible-option-versionadded">added in community.aws 5.3.0</em></p>
 </div></td>
 <td><div class="ansible-option-cell"><p>Toggle overwriting of existing placement strategy. This is needed for backwards compatibility.</p>
-<p>By default <em>purge_placement_strategy=false</em>. In a release after 2024-06-01 this will be changed to <em>purge_placement_strategy=true</em>.</p>
+<p>By default <em>purge_placement_strategy=false</em>. In release 9.0.0 this will be changed to <em>purge_placement_strategy=true</em>.</p>
 <p class="ansible-option-line"><strong class="ansible-option-choices">Choices:</strong></p>
 <ul class="simple">
 <li><p><code class="ansible-option-default-bold docutils literal notranslate"><strong><span class="pre">false</span></strong></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/elb_network_lb_module.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/elb_network_lb_module.html
index b4671f0..0f6998a 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/elb_network_lb_module.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/elb_network_lb_module.html
@@ -457,6 +457,16 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 </div></td>
 </tr>
 <tr class="row-even"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-security_groups"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-security-groups"><strong>security_groups</strong></p>
+<a class="ansibleOptionLink" href="#parameter-security_groups" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">list</span> / <span class="ansible-option-elements">elements=string</span></p>
+<p><em class="ansible-option-versionadded">added in community.aws 7.3.0</em></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>A list of the names or IDs of the security groups to assign to the load balancer.</p>
+<p>Required if <em>state=present</em>.</p>
+<p>If <code class="docutils literal notranslate"><span class="pre">[]</span></code>, the VPC’s default security group will be used.</p>
+</div></td>
+</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-session_token"></div>
 <div class="ansibleOptionAnchor" id="parameter-aws_session_token"></div>
 <div class="ansibleOptionAnchor" id="parameter-security_token"></div>
@@ -474,7 +484,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p>Support for the <code class="docutils literal notranslate"><span class="pre">EC2_SECRET_KEY</span></code> and <code class="docutils literal notranslate"><span class="pre">AWS_SECURITY_TOKEN</span></code> environment variables has been deprecated and will be removed in a release after 2024-12-01.</p>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-state"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-state"><strong>state</strong></p>
 <a class="ansibleOptionLink" href="#parameter-state" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
 </div></td>
@@ -487,7 +497,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 </ul>
 </div></td>
 </tr>
-<tr class="row-even"><td><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-subnet_mappings"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-subnet-mappings"><strong>subnet_mappings</strong></p>
 <a class="ansibleOptionLink" href="#parameter-subnet_mappings" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">list</span> / <span class="ansible-option-elements">elements=dictionary</span></p>
 </div></td>
@@ -495,7 +505,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p>This parameter is mutually exclusive with <em>subnets</em>.</p>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-subnets"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-subnets"><strong>subnets</strong></p>
 <a class="ansibleOptionLink" href="#parameter-subnets" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">list</span> / <span class="ansible-option-elements">elements=string</span></p>
 </div></td>
@@ -504,7 +514,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p>This parameter is mutually exclusive with <em>subnet_mappings</em>.</p>
 </div></td>
 </tr>
-<tr class="row-even"><td><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-tags"></div>
 <div class="ansibleOptionAnchor" id="parameter-resource_tags"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-tags"><span id="ansible-collections-community-aws-elb-network-lb-module-parameter-resource-tags"></span><strong>tags</strong></p>
 <a class="ansibleOptionLink" href="#parameter-tags" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-aliases">aliases: resource_tags</span></p>
@@ -514,7 +524,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p>If the <em>tags</em> parameter is not set then tags will not be modified.</p>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-validate_certs"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-validate-certs"><strong>validate_certs</strong></p>
 <a class="ansibleOptionLink" href="#parameter-validate_certs" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
 </div></td>
@@ -527,7 +537,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 </ul>
 </div></td>
 </tr>
-<tr class="row-even"><td><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-wait"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-wait"><strong>wait</strong></p>
 <a class="ansibleOptionLink" href="#parameter-wait" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
 </div></td>
@@ -539,7 +549,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 </ul>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-wait_timeout"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-parameter-wait-timeout"><strong>wait_timeout</strong></p>
 <a class="ansibleOptionLink" href="#parameter-wait_timeout" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">integer</span></p>
 </div></td>
@@ -840,6 +850,15 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 </div></td>
 </tr>
 <tr class="row-even"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="return-load_balancer/security_groups"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-return-load-balancer-security-groups"><strong>security_groups</strong></p>
+<a class="ansibleOptionLink" href="#return-load_balancer/security_groups" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">list</span> / <span class="ansible-option-elements">elements=string</span></p>
+</div></td>
+<td><div class="ansible-option-indent-desc"></div><div class="ansible-option-cell"><p>The IDs of the security groups for the load balancer.</p>
+<p class="ansible-option-line"><strong class="ansible-option-returned-bold">Returned:</strong> when state is present</p>
+<p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">[&quot;sg-0011223344&quot;]</span></code></p>
+</div></td>
+</tr>
+<tr class="row-odd"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="return-load_balancer/state"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-return-load-balancer-state"><strong>state</strong></p>
 <a class="ansibleOptionLink" href="#return-load_balancer/state" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">dictionary</span></p>
 </div></td>
@@ -848,7 +867,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">{&quot;code&quot;:</span> <span class="pre">&quot;active&quot;}</span></code></p>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="return-load_balancer/tags"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-return-load-balancer-tags"><strong>tags</strong></p>
 <a class="ansibleOptionLink" href="#return-load_balancer/tags" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">dictionary</span></p>
 </div></td>
@@ -857,7 +876,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">{&quot;Tag&quot;:</span> <span class="pre">&quot;Example&quot;}</span></code></p>
 </div></td>
 </tr>
-<tr class="row-even"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="return-load_balancer/type"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-return-load-balancer-type"><strong>type</strong></p>
 <a class="ansibleOptionLink" href="#return-load_balancer/type" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
 </div></td>
@@ -866,7 +885,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-elb-n
 <p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">&quot;network&quot;</span></code></p>
 </div></td>
 </tr>
-<tr class="row-odd"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-indent"></div><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="return-load_balancer/vpc_id"></div><p class="ansible-option-title" id="ansible-collections-community-aws-elb-network-lb-module-return-load-balancer-vpc-id"><strong>vpc_id</strong></p>
 <a class="ansibleOptionLink" href="#return-load_balancer/vpc_id" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
 </div></td>
diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/glue_connection_module.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/glue_connection_module.html
index a311b2a..6f456df 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/glue_connection_module.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/glue_connection_module.html
@@ -467,7 +467,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-glue-
 <a class="ansibleOptionLink" href="#return-connection_properties" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">dictionary</span></p>
 </div></td>
 <td><div class="ansible-option-cell"><p>(deprecated) A dict of key-value pairs (converted to lowercase) used as parameters for this connection.</p>
-<p>This return key has been deprecated, and will be removed in a release after 2024-06-01.</p>
+<p>This return key has been deprecated, and will be removed in release 9.0.0.</p>
 <p class="ansible-option-line"><strong class="ansible-option-returned-bold">Returned:</strong> when state is present</p>
 <p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">{&quot;jdbc_connection_url&quot;:</span> <span class="pre">&quot;jdbc:mysql://mydb:3306/databasename&quot;,</span> <span class="pre">&quot;password&quot;:</span> <span class="pre">&quot;y&quot;,</span> <span class="pre">&quot;username&quot;:</span> <span class="pre">&quot;x&quot;}</span></code></p>
 </div></td>

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/3c71d9e33b6a4eef8bba38af3f5bf531

ansible-galaxy-importer FAILURE in 5m 11s (non-voting)
✔️ build-ansible-collection SUCCESS in 14m 49s
✔️ ansible-test-splitter SUCCESS in 5m 35s
integration-community.aws-1 RETRY_LIMIT in 1m 51s
Skipped 21 jobs

@markuman
Copy link
Member

That's interesting. In the past it was not possible to use security groups on network loadbalancer.
Our existing said

No security group associated
Because this load balancer was created without a security group, these settings can't be changed. To utilize security groups, ensure that one is specified during creation of the load balancer.

I guess that must be also documentated, ...that a new one is necessary, in case someone wanted to use this.

@markuman
Copy link
Member

@erinkdelong You need to add a changelog fragment changelogs/fragments.

erinkdelong and others added 2 commits April 30, 2024 21:17
@erinkdelong
Copy link
Author

@erinkdelong You need to add a changelog fragment changelogs/fragments.

Thanks! I just added it. Sorry for the delay, I've been super busy.

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/95be38c7785b47a0a00337b3cdf0fad3

✔️ ansible-galaxy-importer SUCCESS in 5m 13s (non-voting)
✔️ build-ansible-collection SUCCESS in 15m 30s
✔️ ansible-test-splitter SUCCESS in 7m 12s
integration-community.aws-1 FAILURE in 7m 37s
Skipped 21 jobs


connection = module.client("elbv2")
connection_ec2 = module.client("ec2")

elb = NetworkLoadBalancer(connection, connection_ec2, module)
# elb = NetworkLoadBalancer(connection, connection_ec2, module)
elb = NetworkLoadBalancerWithSecurityGroups(connection, connection_ec2, module)
Copy link
Member

Choose a reason for hiding this comment

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

something is broken here @erinkdelong

File "/tmp/ansible_elb_network_lb_payload_wfua5ziu/ansible_elb_network_lb_payload.zip/ansible_collections/community/aws/plugins/modules/elb_network_lb.py", line 647, in
File "/tmp/ansible_elb_network_lb_payload_wfua5ziu/ansible_elb_network_lb_payload.zip/ansible_collections/community/aws/plugins/modules/elb_network_lb.py", line 638, in main
File "/tmp/ansible_elb_network_lb_payload_wfua5ziu/ansible_elb_network_lb_payload.zip/ansible_collections/community/aws/plugins/modules/elb_network_lb.py", line 477, in init
TypeError: NetworkLoadBalancer.init() missing 1 required positional argument: 'module'

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/28acfce0b4ee458d98865ad1a77770b0

ansible-galaxy-importer FAILURE in 5m 44s (non-voting)
✔️ build-ansible-collection SUCCESS in 15m 10s
✔️ ansible-test-splitter SUCCESS in 6m 27s
✔️ integration-community.aws-1 SUCCESS in 19m 51s
Skipped 21 jobs

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/683514553d934cdda59e2353a731fdc3

✔️ ansible-galaxy-importer SUCCESS in 5m 51s (non-voting)
✔️ build-ansible-collection SUCCESS in 15m 35s
✔️ ansible-test-splitter SUCCESS in 6m 09s
integration-community.aws-1 FAILURE in 33m 33s
Skipped 21 jobs

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.

None yet

2 participants