-
Notifications
You must be signed in to change notification settings - Fork 341
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
[WIP] ec2_vpc_endpoint_service module #1573
base: main
Are you sure you want to change the base?
[WIP] ec2_vpc_endpoint_service module #1573
Conversation
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 07s |
Docs Build 📝Thank you for contribution!✨ The docsite for this PR is available for download as an artifact from this run: You can compare to the docs for the File changes:
Click to see the diff comparison.NOTE: only file modifications are shown here. New and deleted files are excluded. diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vpc_endpoint_info_module.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vpc_endpoint_info_module.html
index ede5935..a5be2d6 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vpc_endpoint_info_module.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vpc_endpoint_info_module.html
@@ -21,7 +21,7 @@
<script src="../../../_static/sphinx_highlight.js?v=dc90522c"></script>
<script src="../../../_static/js/theme.js"></script>
<link rel="search" title="Search" href="../../../search.html" />
- <link rel="next" title="amazon.aws.ec2_vpc_endpoint_service_info module – Retrieves AWS VPC endpoint service details" href="ec2_vpc_endpoint_service_info_module.html" />
+ <link rel="next" title="amazon.aws.ec2_vpc_endpoint_service module – create, delete VPC endpoint service" href="ec2_vpc_endpoint_service_module.html" />
<link rel="prev" title="amazon.aws.ec2_vpc_endpoint module – Create and delete AWS VPC endpoints" href="ec2_vpc_endpoint_module.html" /><!-- extra head elements for Ansible beyond RTD Sphinx Theme -->
@@ -578,7 +578,7 @@ see <a class="reference internal" href="#ansible-collections-amazon-aws-ec2-vpc-
<footer><div class="rst-footer-buttons" role="navigation" aria-label="Footer">
<a href="ec2_vpc_endpoint_module.html" class="btn btn-neutral float-left" title="amazon.aws.ec2_vpc_endpoint module – Create and delete AWS VPC endpoints" accesskey="p" rel="prev"><span class="fa fa-arrow-circle-left" aria-hidden="true"></span> Previous</a>
- <a href="ec2_vpc_endpoint_service_info_module.html" class="btn btn-neutral float-right" title="amazon.aws.ec2_vpc_endpoint_service_info module – Retrieves AWS VPC endpoint service details" accesskey="n" rel="next">Next <span class="fa fa-arrow-circle-right" aria-hidden="true"></span></a>
+ <a href="ec2_vpc_endpoint_service_module.html" class="btn btn-neutral float-right" title="amazon.aws.ec2_vpc_endpoint_service module – create, delete VPC endpoint service" accesskey="n" rel="next">Next <span class="fa fa-arrow-circle-right" aria-hidden="true"></span></a>
</div>
<hr/>
diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vpc_endpoint_service_info_module.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vpc_endpoint_service_info_module.html
index 4d695b1..d2392da 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vpc_endpoint_service_info_module.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vpc_endpoint_service_info_module.html
@@ -22,7 +22,7 @@
<script src="../../../_static/js/theme.js"></script>
<link rel="search" title="Search" href="../../../search.html" />
<link rel="next" title="amazon.aws.ec2_vpc_igw module – Manage an AWS VPC Internet gateway" href="ec2_vpc_igw_module.html" />
- <link rel="prev" title="amazon.aws.ec2_vpc_endpoint_info module – Retrieves AWS VPC endpoints details using AWS methods" href="ec2_vpc_endpoint_info_module.html" /><!-- extra head elements for Ansible beyond RTD Sphinx Theme -->
+ <link rel="prev" title="amazon.aws.ec2_vpc_endpoint_service module – create, delete VPC endpoint service" href="ec2_vpc_endpoint_service_module.html" /><!-- extra head elements for Ansible beyond RTD Sphinx Theme -->
@@ -511,7 +511,7 @@ see <a class="reference internal" href="#ansible-collections-amazon-aws-ec2-vpc-
<footer><div class="rst-footer-buttons" role="navigation" aria-label="Footer">
- <a href="ec2_vpc_endpoint_info_module.html" class="btn btn-neutral float-left" title="amazon.aws.ec2_vpc_endpoint_info module – Retrieves AWS VPC endpoints details using AWS methods" accesskey="p" rel="prev"><span class="fa fa-arrow-circle-left" aria-hidden="true"></span> Previous</a>
+ <a href="ec2_vpc_endpoint_service_module.html" class="btn btn-neutral float-left" title="amazon.aws.ec2_vpc_endpoint_service module – create, delete VPC endpoint service" accesskey="p" rel="prev"><span class="fa fa-arrow-circle-left" aria-hidden="true"></span> Previous</a>
<a href="ec2_vpc_igw_module.html" class="btn btn-neutral float-right" title="amazon.aws.ec2_vpc_igw module – Manage an AWS VPC Internet gateway" accesskey="n" rel="next">Next <span class="fa fa-arrow-circle-right" aria-hidden="true"></span></a>
</div>
diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/index.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/index.html
index fc42e83..2f7dc0a 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/index.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/index.html
@@ -149,6 +149,7 @@
</nav>
<section id="description">
<h2><a class="toc-backref" href="#id1" role="doc-backlink">Description</a><a class="headerlink" href="#description" title="Link to this heading"></a></h2>
+<p>A variety of Ansible content to help automate the management of AWS services.</p>
<p><strong>Author:</strong></p>
<ul class="simple">
<li><p>Ansible (<a class="reference external" href="https://github.com/ansible">https://github.com/ansible</a>)</p></li>
@@ -262,6 +263,7 @@
<li><p><a class="reference internal" href="ec2_vpc_dhcp_option_info_module.html#ansible-collections-amazon-aws-ec2-vpc-dhcp-option-info-module"><span class="std std-ref">ec2_vpc_dhcp_option_info module</span></a> – Gather information about DHCP options sets in AWS</p></li>
<li><p><a class="reference internal" href="ec2_vpc_endpoint_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-module"><span class="std std-ref">ec2_vpc_endpoint module</span></a> – Create and delete AWS VPC endpoints</p></li>
<li><p><a class="reference internal" href="ec2_vpc_endpoint_info_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-info-module"><span class="std std-ref">ec2_vpc_endpoint_info module</span></a> – Retrieves AWS VPC endpoints details using AWS methods</p></li>
+<li><p><a class="reference internal" href="ec2_vpc_endpoint_service_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-service-module"><span class="std std-ref">ec2_vpc_endpoint_service module</span></a> – create, delete VPC endpoint service</p></li>
<li><p><a class="reference internal" href="ec2_vpc_endpoint_service_info_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-service-info-module"><span class="std std-ref">ec2_vpc_endpoint_service_info module</span></a> – Retrieves AWS VPC endpoint service details</p></li>
<li><p><a class="reference internal" href="ec2_vpc_igw_module.html#ansible-collections-amazon-aws-ec2-vpc-igw-module"><span class="std std-ref">ec2_vpc_igw module</span></a> – Manage an AWS VPC Internet gateway</p></li>
<li><p><a class="reference internal" href="ec2_vpc_igw_info_module.html#ansible-collections-amazon-aws-ec2-vpc-igw-info-module"><span class="std std-ref">ec2_vpc_igw_info module</span></a> – Gather information about internet gateways in AWS</p></li>
diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/index_module.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/index_module.html
index 1ab942b..8a4e620 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/index_module.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/index_module.html
@@ -172,6 +172,7 @@
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_dhcp_option_info_module.html#ansible-collections-amazon-aws-ec2-vpc-dhcp-option-info-module"><span class="std std-ref">amazon.aws.ec2_vpc_dhcp_option_info</span></a> – Gather information about DHCP options sets in AWS</p></li>
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_endpoint_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-module"><span class="std std-ref">amazon.aws.ec2_vpc_endpoint</span></a> – Create and delete AWS VPC endpoints</p></li>
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_endpoint_info_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-info-module"><span class="std std-ref">amazon.aws.ec2_vpc_endpoint_info</span></a> – Retrieves AWS VPC endpoints details using AWS methods</p></li>
+<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_endpoint_service_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-service-module"><span class="std std-ref">amazon.aws.ec2_vpc_endpoint_service</span></a> – create, delete VPC endpoint service</p></li>
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_endpoint_service_info_module.html#ansible-collections-amazon-aws-ec2-vpc-endpoint-service-info-module"><span class="std std-ref">amazon.aws.ec2_vpc_endpoint_service_info</span></a> – Retrieves AWS VPC endpoint service details</p></li>
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_igw_module.html#ansible-collections-amazon-aws-ec2-vpc-igw-module"><span class="std std-ref">amazon.aws.ec2_vpc_igw</span></a> – Manage an AWS VPC Internet gateway</p></li>
<li><p><a class="reference internal" href="amazon/aws/ec2_vpc_igw_info_module.html#ansible-collections-amazon-aws-ec2-vpc-igw-info-module"><span class="std std-ref">amazon.aws.ec2_vpc_igw_info</span></a> – Gather information about internet gateways in AWS</p></li>
|
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.
Thanks for taking the time to submit this PR.
Various comments inline.
Additionally,
- Please run this code the the black formatter (https://github.com/psf/black)
- Please add some integration tests (tests/integration/targets) https://github.com/ansible-collections/amazon.aws/blob/main/docs/docsite/rst/dev_guidelines.rst#integration-tests-for-aws-modules
if tags: | ||
params["TagSpecifications"] = [{'ResourceType': 'vpc-endpoint-service', 'Tags': ansible_dict_to_boto3_tag_list(tags)}] |
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.
ansible_collections.amazon.aws.plugins.module_utils.tagging.boto3_tag_specifications
can generate these for you.
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 part of code I took again from backup_plan, so first I need to understand what the "poet means" and adopt
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.
The AWS APIs have a rather weird way of specifying Tags (and they're not even consistent)
Let's say you wanted to apply the following tags (in usual Key/Value format
{
"MyFirstKey": "My first value",
"my_second_key": "another value",
)
There are two ways the EC2 API want you to pass this
- A "list' of tags" aka
boto3_tag_list
or list-of-dicts (mostly when updating tags):
[
{
"Key": "MyFirstKey",
"Value": "My first value",
},
{
"Key": "my_second_key",
"Value": "another value",
}
]
TagSpecifications
- a mapping of resource types to a boto3_tag_list (mostly when creating 1 or more resources). This seems really strange, but some of the create calls create multiple types of resource, this lets you specify different tags for each type of resource
[
{
"ResourceType": "vpc-endpoint-service",
"Tags": [
{
"Key": "MyFirstKey",
"Value": "My first value",
},
{
"Key": "my_second_key",
"Value": "another value",
}
]
}
]
These can be something of a PITA to work with, and Amazon's not even consistent with the formats it uses :(
However, we have some helpers to try and make this easier to deal with.
- ansible_dict_to_boto3_tag_list - Takes a simple dictionary and converts it to the
boto3_tag_list
format - boto3_tag_list_to_ansible_dict - The other way around
- boto3_tag_specifications - Takes a list of resource types and a simply dictionary representing the tags, then returns the
TagSpecifications
format.
Separately we have ensure_ec2_tags
, which takes a resource-id, a normal dict (for the tags), and purge. It handles the format conversions, and knows how/when to add/remove tags based on the purge setting. (but only for EC2 resource)
What gets really messy is that not all of Amazon's APIs even use the same formats: some use simple dicts, some the above format, and some even use list-of-dicts change "Key" and "Value"
except ImportError: | ||
pass # Handled by AnsibleAWSModule | ||
|
||
ARGUMENT_SPEC = dict( |
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.
I'd probably avoid setting defaults for most of the options here (except state). While default behaviour on creation should be documented, when you're performing an update it's not possible to tell the difference between defaults and user-requested changes. In general we should only change a parameter if someone explicitly passed that parameter.
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.
So the logic would be that there is no defaults set on module level, but we manage None
s coming from user side when we need to create/update? (around lines 443 and 470)
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.
yes.
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.
I'd probably avoid setting defaults for most of the options here (except state). While default behaviour on creation should be documented, when you're performing an update it's not possible to tell the difference between defaults and user-requested changes. In general we should only change a parameter if someone explicitly passed that parameter.
Is it ok if in the documentation and in ARGUMENT_SPEC I specify None as default for these parameters mentioning that defaults will be set as per boto3?
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.
That's fine (although it's the remote AWS service that'll set the default rather than the client SDK)
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.
updated
@krisek in addition to the comments above, there's a number of failing tests. Please let us know if you need help fixing the issues they raise. |
Build failed. ❌ ansible-galaxy-importer FAILURE in 3m 53s |
Build failed. ❌ ansible-galaxy-importer FAILURE in 3m 56s |
recheck |
Build failed. ❌ ansible-galaxy-importer FAILURE in 5m 01s |
@krisek this PR contains the following merge commits: Please rebase your branch to remove these commits. |
@krisek This PR was evaluated as a potentially problematic PR for the following reasons:
Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: |
Build failed. ❌ ansible-galaxy-importer FAILURE in 3m 40s |
I'm sorry it takes so long, I lost a bit focus in the recent months. As the branch slipped behind upstream... shall I re-raise it from a new fork? |
Try using |
😰 when I hear force push 🙂 but I'll give a try |
Rebasing and force pushing is nothing to be afraid of with feature branches (certainly to be avoided with |
Co-authored-by: Mark Chappell <[email protected]>
Co-authored-by: Mark Chappell <[email protected]>
Co-authored-by: Mark Chappell <[email protected]>
Co-authored-by: Mark Chappell <[email protected]>
Co-authored-by: Mark Chappell <[email protected]>
Co-authored-by: Mark Chappell <[email protected]>
a806477
to
debcdcb
Compare
Build failed. ❌ ansible-galaxy-importer FAILURE in 4m 58s |
local |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 41s |
Before removing WIP, I guess I should add integration tests... can somebody please suggest where to start? |
…le-collections#1575) ssm_parameter: add support for tags (ansible-collections#1573) SUMMARY Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request ansible-collections#1573. ISSUE TYPE Feature Pull Request COMPONENT NAME ssm_parameter ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Haskell (mikehas) <None> Reviewed-by: Dennis Qian <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
…le-collections#1575) ssm_parameter: add support for tags (ansible-collections#1573) SUMMARY Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request ansible-collections#1573. ISSUE TYPE Feature Pull Request COMPONENT NAME ssm_parameter ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Haskell (mikehas) <None> Reviewed-by: Dennis Qian <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
…le-collections#1575) ssm_parameter: add support for tags (ansible-collections#1573) SUMMARY Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request ansible-collections#1573. ISSUE TYPE Feature Pull Request COMPONENT NAME ssm_parameter ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Haskell (mikehas) <None> Reviewed-by: Dennis Qian <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
SUMMARY
Module to create, update, or delete AWS Endpoint Services
ISSUE TYPE
COMPONENT NAME
ec2_vpc_endpoint_service
ADDITIONAL INFORMATION
Please see module documentation