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

Define a naming scheme for AWS modules #610

Closed
1 task done
tremble opened this issue Jan 12, 2022 · 13 comments · Fixed by #881
Closed
1 task done

Define a naming scheme for AWS modules #610

tremble opened this issue Jan 12, 2022 · 13 comments · Fixed by #881
Labels
feature This issue/PR relates to a feature request

Comments

@tremble
Copy link
Contributor

tremble commented Jan 12, 2022

Summary

Most of the history with the aws_ prefix comes from before Ansible split most of the content out into collections. At that point in time there was no practical namespace for modules to live in, as a result prefixes like aws_ were used to avoid a collision. Take for example the aws_secret module, without an aws_ prefix, there's a massive array of secret stores in the Ansible universe that this might have referred to, and first-come-first-served is a poor way to deal with it.

Now that we have the amazon.aws and community.aws namespaces that modules can live in we could start cleaning up the module names. For this to happen someone would need to

a) Propose a naming scheme which takes into account things like multiple services having a snapshot
b) Put together a PR to document said standard (probably link to an amazon.aws doc from the community.aws docs too)
c) (eventually) Put together the relevant pull requests

While I don't think "RightNow" would be a good time to do a big-bang bulk migration (will there ever be one), at least one driven solely by the Cloud Content team. I do think that defining a standard sooner rather than later offers some clarity for folks submitting new modules to community.aws. Additionally, as and when modules are migrated from community.aws to amazon.aws we can opportunistically update module names.

Once we have a standard, should someone from the community want to spend time renaming groups of modules this doesn't have to be driven by the Cloud Content team (meta/runtime.yml makes this so much easier).

Issue Type

Feature Idea

Component Name

plugins/
docs/

Additional Information

CC: @markuman

For an example of the confusion, see also ansible-collections/community.aws#858

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@tremble
Copy link
Contributor Author

tremble commented Jan 12, 2022

Personally I'd recommend that we keep the service name (rds_/ec2_/elb_/iam_) as a prefix, because there's a lot of overlap in resource naming (snapshots, security groups, and instances springs to mind), but I wouldn't differentiate between elb and elbv2. The aws_ prefix I'd probably reserve for 'global' AWS things like aws_region_info.

The OpenStack folks did a big rename using a similar scheme. However they dropped the service prefix for 'User' facing things vs 'Admin' facing things, with Amazon that's a much fuzzier line. https://review.opendev.org/c/openstack/ansible-collections-openstack/+/726538

@markuman
Copy link
Member

For an example of the confusion, see also ansible-collections/community.aws#858

Does community.aws.opensearch feels confusing to you? The initial suggestion comes from @jillr and I agree :)
Because opensearch is a separate service, it does not fit into a prefix ec2_ etc.

But in general I agree with your suggestion to keep (rds_/ec2_/elb_/iam_) for most modules.
And it would be good to have a guideline about module names, so it is less of a stomach decision in the future.

One point of contention might be amazon.aws.ec2_group, because security groups are not exclusive to the ec2 Service (virtual servers).
They are used in many services (vpc client vpn, rds, elasticache, lambda...). And "pyhsically" they are always attached to an ENI.
I've got no answer to this, but ec2_group does not feel correct to me.

The aws_ prefix I'd probably reserve for 'global' AWS things like aws_region_info.

Maybe aws_security_group ... just brainstorming...

@tremble
Copy link
Contributor Author

tremble commented Jan 12, 2022

I don't find the result of the discussion, ie community.aws.opensearch, to be confusing (I would have commented there if I did). The confusion I was referring to was that the person who raised the issue initially suggested ec2_opensearch. Given that OpenSearch is technically its own service rather than being part of the EC2 APIs, this implies to me that the original suggestion of ec2_opensearch (and side comments about the aws_ prefix) was based on some form of confusion, likely due to a lack of consistency and documentation. That's what I'd like to see corrected.

Specifically with regards security groups, they are a part of the EC2 APIs, so while they're also consumed by non-EC2 services I think it's "technically" correct to refer to them with an ec2_ prefix. Some of the services even have their own, separate, "security group" implementations in their APIs: RedShift Security Groups for example.

I mentioned security groups because we could for example simply call the module amazon.aws.security_group, since we now have the amazon.aws and community.aws namespaces it's not necessary to try and avoid a collision between (for example) an OpenStack security_group module and an AWS security_group module: it's now possible to explicitly reference the modules as openstack.cloud.security_group and amazon.aws.security_group. Unfortunately, because Amazon has things like "security groups" where they reuse the name, but offer multiple interfaces, I think we'd be better off keeping the prefix ec2_security_group vs redshift_security_group. For what it's worth I don't like it being just "ec2_group", I'd rather "ec2_security_group", because "Instance Groups" are also now a thing and ec2_group could itself lead to confusion.

That all said, this is exactly the kind of discussion I was trying to start. My first comment was an initial "straw-man" proposal, if you can pick holes in it, then perfect, we can build something better together :)

@markuman
Copy link
Member

ansible-collections/community.aws#887

More candidates

  • aws_codepipeline
  • aws_codebuild
  • aws_codecommit

Imo, the aws_ prefix should be removed and since the service names are CodePipeline, CodeBuild, CodeCommit, CodeDeploy, convert to snake dict would result in code_ prefix

  • community.aws.code_pipeline
  • community.aws.code_build
  • community.aws.code_commit
  • community.aws.code_deploy

What do you think?

@tremble
Copy link
Contributor Author

tremble commented Jan 27, 2022

Looking at how AWS have structured their APIs There are a set of separate APIs, each with sets of resources under them:

codeartifact
codebuild
codecommit
codedeploy
codepipeline

I would suggest something more like:

community.aws.codedeploy_deployment
community.aws.codedeploy_deployment_group

There are also a few things around like
ses / sesv2
elb / elbv2

I'd suggest something like dropping the v2 where this happens, the UIs often hide the v1/v2 distinction/

@alinabuzachis
Copy link
Collaborator

For an example of the confusion, see also ansible-collections/community.aws#858

Does community.aws.opensearch feels confusing to you? The initial suggestion comes from @jillr and I agree :) Because opensearch is a separate service, it does not fit into a prefix ec2_ etc.

But in general I agree with your suggestion to keep (rds_/ec2_/elb_/iam_) for most modules. And it would be good to have a guideline about module names, so it is less of a stomach decision in the future.

One point of contention might be amazon.aws.ec2_group, because security groups are not exclusive to the ec2 Service (virtual servers). They are used in many services (vpc client vpn, rds, elasticache, lambda...). And "pyhsically" they are always attached to an ENI. I've got no answer to this, but ec2_group does not feel correct to me.

The aws_ prefix I'd probably reserve for 'global' AWS things like aws_region_info.

Maybe aws_security_group ... just brainstorming...

I personally find aws_security_group confusing because of the aws_ prefix. I will go more for ec2_security_group (but this is my personal feeling). I throw it there, what about organising the modules in subfolders, where each subfolder is named according to the macro catagory (don't know if this is feasible). For example: for rds - [plugins/modules/rds/instance.py, plugins/modules/rds/cluster.py, plugins/modules/rds/subnet_group.py,..., etc], for ec2 - [plugins/modules/ec2/security_group.py, plugins/modules/ec2/instance.py,..., etc]

@markuman
Copy link
Member

I'd suggest something like dropping the v2 where this happens, the UIs often hide the v1/v2 distinction/

That's not that easy I think. For example wafv2 and waf are completely different.
Yes, the GUI is hiding it. But it's a tightrope walk. On the one hand, ansible is a layer that can hide complexity. But on the other hand, you cannot use AWS everywhere without some deeper knowledge.

@markuman
Copy link
Member

Looking at how AWS have structured their APIs There are a set of separate APIs, each with sets of resources under them:Looking at how AWS have structured their APIs There are a set of separate APIs, each with sets of resources under them:

+1

codeartifact
codebuild
codecommit
codedeploy
codepipeline

I would suggest something more like:

community.aws.codedeploy_deployment
community.aws.codedeploy_deployment_group

I'm fine with that.

@tremble
Copy link
Contributor Author

tremble commented Jan 27, 2022

I throw it there, what about organising the modules in subfolders, where each subfolder is named according to the macro catagory (don't know if this is feasible).

I like the idea as far as sorting things is concerned. Do you know if we could reflect this in the FQCN?

@alinabuzachis
Copy link
Collaborator

I throw it there, what about organising the modules in subfolders, where each subfolder is named according to the macro catagory (don't know if this is feasible).

I like the idea as far as sorting things is concerned. Do you know if we could reflect this in the FQCN?

Not really. I'll try to document.

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request needs_triage labels Jan 27, 2022
@tremble tremble mentioned this issue Jan 27, 2022
1 task
@jillr
Copy link
Collaborator

jillr commented Jan 27, 2022

Thanks @tremble for kicking this off! I concur with generally removing the aws_ prefixes from things like aws_s3, but keeping them in specific instances where it makes sense (aws_region_info and aws_caller_info are the only ones that stand out to me now).

While FQCN's do allow us to have module name collisions and FQCN's are the preferred / recommended way to write tasks, I'd still be hesitant to have things that are too generic without a service prefix (like your example of snapshot) because we can assume lots of users will still use short names in tasks and we can reasonably predict it could cause confusion.

I'm also leery of a having big rename of existing modules given that users have already had to modify playbooks recently for facts.py->info.py, the collections break out, various deprecations and module migrations, etc. We need to be respectful of the impact this all has had on them. I'm a big +1 for adopting a standard going forward for new things, and planning to retrofit it onto existing modules, but we should be thoughtful about when and how we do that.

Once we do have a something close to a draft proposal, before we formally adopt it, we should run it past the rest of the cloud content team and Ansible product management. If we're investing the time in standards development there might be opportunities to synchronize across collections if we include a broader group.

@tremble
Copy link
Contributor Author

tremble commented Jan 28, 2022

(aws_region_info and aws_caller_info are the only ones that stand out to me now)

Agreed, the only other one that I can think of is the aws_account_attribute lookup plugin.

While FQCN's do allow us to have module name collisions and FQCN's are the preferred / recommended way to write tasks, I'd still be hesitant to have things that are too generic without a service prefix (like your example of snapshot) because we can assume lots of users will still use short names in tasks and we can reasonably predict it could cause confusion.

Yeah, the other one that worries me is the secrets manager service, which only has a single resource type secrets. We either end up with secretsmanager_secret, which feels redundant, or secret which feels generic enough that collisions are almost guaranteed to happen. Currently it's aws_secret

I'm also leery of a having big rename of existing modules given that users have already had to modify playbooks recently for facts.py->info.py, the collections break out, various deprecations and module migrations, etc. We need to be respectful of the impact this all has had on them. I'm a big +1 for adopting a standard going forward for new things, and planning to retrofit it onto existing modules, but we should be thoughtful about when and how we do that.

Yeah, I've been on the customer side of that too :)

However, in some cases the current names make it harder to find the modules that you want. Having a consistent naming scheme makes things much more discoverable.

In amazon.aws the two modules I'd rename are

  • ec2_group - Personally I find this too generic and confusing: there are a lot of groups of EC2 resources, I'd rename it ec2_security_group
  • aws_s3 - Again this is confusing. We have s3_bucket, for managing the bucket itself, and aws_s3 for managing the objects inside the bucket. I'd rename it s3_object.

However, what I wouldn't do is set a deprecation timeline for the old names, as you mention there's been a lot of churn in the last couple of years. If we can make it easier to find the modules people are looking for, then I think that brings more value than the cost of carrying some extra aliases.

The vast bulk of modules that IMHO need renaming are over in community.aws. If we can pick a new naming scheme, then as they're promoted into amazon.aws we can leave the 'old' alias behind in community.aws while only using the new name in amazon.aws.

@tremble tremble pinned this issue Jan 30, 2022
@jillr
Copy link
Collaborator

jillr commented Jan 31, 2022

That plan feels sensible to me, +1

@jillr jillr removed the needs_triage label Feb 8, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this issue Jun 24, 2022
Module naming guidelines

SUMMARY
Originally discussed in #610
Here's an initial attempt at some module naming (and module scoping) guidelines.
Assuming we adopt this, I recommend is that:

we can do the renames as part of 5.0.0, but this isn't a must and wouldn't be considered a blocker to 5.0.0 unless otherwise agreed.
any modules moving from community.aws to amazon.aws would be renamed if appropriate as part of the move.
we do not specify a deprecation date at this time, but leave a comment in the redirect that any deprecation cycle should not start before 2024-09-01 (to avoid potential migration fatigue issues),

ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
fixes: #610

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
patchback bot pushed a commit that referenced this issue Jun 24, 2022
Module naming guidelines

SUMMARY
Originally discussed in #610
Here's an initial attempt at some module naming (and module scoping) guidelines.
Assuming we adopt this, I recommend is that:

we can do the renames as part of 5.0.0, but this isn't a must and wouldn't be considered a blocker to 5.0.0 unless otherwise agreed.
any modules moving from community.aws to amazon.aws would be renamed if appropriate as part of the move.
we do not specify a deprecation date at this time, but leave a comment in the redirect that any deprecation cycle should not start before 2024-09-01 (to avoid potential migration fatigue issues),

ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
fixes: #610

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit 289e987)
tremble added a commit that referenced this issue Jun 24, 2022
Module naming guidelines

SUMMARY
Originally discussed in #610
Here's an initial attempt at some module naming (and module scoping) guidelines.
Assuming we adopt this, I recommend is that:

we can do the renames as part of 5.0.0, but this isn't a must and wouldn't be considered a blocker to 5.0.0 unless otherwise agreed.
any modules moving from community.aws to amazon.aws would be renamed if appropriate as part of the move.
we do not specify a deprecation date at this time, but leave a comment in the redirect that any deprecation cycle should not start before 2024-09-01 (to avoid potential migration fatigue issues),

ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
fixes: #610

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit 289e987)
softwarefactory-project-zuul bot pushed a commit that referenced this issue Jun 24, 2022
[PR #881/289e9870 backport][stable-4] Module naming guidelines

This is a backport of PR #881 as merged into main (289e987).
SUMMARY
Originally discussed in #610
Here's an initial attempt at some module naming (and module scoping) guidelines.
Assuming we adopt this, I recommend is that:

we can do the renames as part of 5.0.0, but this isn't a must and wouldn't be considered a blocker to 5.0.0 unless otherwise agreed.
any modules moving from community.aws to amazon.aws would be renamed if appropriate as part of the move.
we do not specify a deprecation date at this time, but leave a comment in the redirect that any deprecation cycle should not start before 2024-09-01 (to avoid potential migration fatigue issues),

ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
fixes: #610

Reviewed-by: Mark Chappell <None>
@tremble tremble unpinned this issue Jun 25, 2022
softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.aws that referenced this issue Jun 25, 2022
Rename ACM modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections/amazon.aws#881 and ansible-collections/amazon.aws#610
Rename aws_acm to acm_certificate
Rename aws_acm_info to acm_certificate_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_acm
aws_acm_info
acm_certificate
acm_certificate_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>
jatorcasso pushed a commit to jatorcasso/amazon.aws that referenced this issue Jun 27, 2022
Module naming guidelines

SUMMARY
Originally discussed in ansible-collections#610
Here's an initial attempt at some module naming (and module scoping) guidelines.
Assuming we adopt this, I recommend is that:

we can do the renames as part of 5.0.0, but this isn't a must and wouldn't be considered a blocker to 5.0.0 unless otherwise agreed.
any modules moving from community.aws to amazon.aws would be renamed if appropriate as part of the move.
we do not specify a deprecation date at this time, but leave a comment in the redirect that any deprecation cycle should not start before 2024-09-01 (to avoid potential migration fatigue issues),

ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
fixes: ansible-collections#610

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.aws that referenced this issue Jun 28, 2022
Rename SES modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections/amazon.aws#881 and ansible-collections/amazon.aws#610
Rename ses modules to remove the aws_ prefix.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_ses_identity.py
plugins/modules/aws_ses_identity_policy.py
plugins/modules/aws_ses_rule_set.py
plugins/modules/ses_identity.py
plugins/modules/ses_identity_policy.py
plugins/modules/ses_rule_set.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename ACM modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename aws_acm to acm_certificate
Rename aws_acm_info to acm_certificate_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_acm
aws_acm_info
acm_certificate
acm_certificate_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename SES modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename ses modules to remove the aws_ prefix.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_ses_identity.py
plugins/modules/aws_ses_identity_policy.py
plugins/modules/aws_ses_rule_set.py
plugins/modules/ses_identity.py
plugins/modules/ses_identity_policy.py
plugins/modules/ses_rule_set.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename ACM modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename aws_acm to acm_certificate
Rename aws_acm_info to acm_certificate_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_acm
aws_acm_info
acm_certificate
acm_certificate_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename SES modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename ses modules to remove the aws_ prefix.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_ses_identity.py
plugins/modules/aws_ses_identity_policy.py
plugins/modules/aws_ses_rule_set.py
plugins/modules/ses_identity.py
plugins/modules/ses_identity_policy.py
plugins/modules/ses_rule_set.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 26, 2023
Rename ACM modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename aws_acm to acm_certificate
Rename aws_acm_info to acm_certificate_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_acm
aws_acm_info
acm_certificate
acm_certificate_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ba08f90
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 27, 2023
Rename ACM modules

SUMMARY
In line with what I understood to be the consensus on ansible-collections#881 and ansible-collections#610
Rename aws_acm to acm_certificate
Rename aws_acm_info to acm_certificate_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_acm
aws_acm_info
acm_certificate
acm_certificate_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ba08f90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants