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

aws_secret - Support purge_tags #1150

Conversation

tremble
Copy link
Contributor

@tremble tremble commented May 19, 2022

SUMMARY

aws_secret currently defaults to purging all tags (even if tags isn't specified), this is a little aggressive.

  1. Add purge_tags parameter
  2. Only purge tags if tags: {} is set (rather than when tags is None
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

aws_secret

ADDITIONAL INFORMATION

Related to #1146

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels May 19, 2022
@tremble tremble changed the title [WIPSupport purge_tags [WIP] aws_secret - Support purge_tags May 19, 2022
@github-actions
Copy link

github-actions bot commented May 19, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@ansibullbot ansibullbot added WIP Work in progress feature This issue/PR relates to a feature request labels May 19, 2022
@softwarefactory-project-zuul

This comment was marked as outdated.

@tremble tremble changed the title [WIP] aws_secret - Support purge_tags aws_secret - Support purge_tags May 20, 2022
@ansibullbot ansibullbot removed the WIP Work in progress label May 20, 2022
@tremble tremble requested a review from markuman May 20, 2022 07:15
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 5m 16s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 57s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 30s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 28s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 42s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 9m 49s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 04s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 34s
✔️ ansible-test-splitter SUCCESS in 2m 31s
✔️ integration-community.aws-1 SUCCESS in 8m 00s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble
Copy link
Contributor Author

tremble commented May 20, 2022

While the integration tests could be further expanded for the non-tag parameters, I'm going to leave them as-is for now. If someone makes other changes to other parameters then we can get them to expand them for that parameter.

Comment on lines -40 to -43
- result.arn is not none
- result.name is not none
- result.tags is not none
- result.version_ids_to_stages is not none
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't actually doing what the author thought it was doing. The values were actually all in result.secret

@@ -139,6 +153,29 @@
returned: always
type: dict
sample: { "dc1ed59b-6d8e-4450-8b41-536dfe4600a9": [ "AWSCURRENT" ] }
tags:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we also need to add version_added also for this returned parameter (even it's not new)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought was no. It's been there since the start of the collection, it just wasn't documented.

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label May 20, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 4m 01s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 07s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 11s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 14m 02s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 9m 46s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 19s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 33s
✔️ ansible-test-splitter SUCCESS in 2m 53s
✔️ integration-community.aws-1 SUCCESS in 7m 05s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit bf4b79f into ansible-collections:main May 20, 2022
@tremble tremble deleted the aws_secret/purge_tags branch July 7, 2022 19:24
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…collections#1150)

ec2_instance: fix to handle create instance in specified AZ

SUMMARY

This PR adds a fix to allow launching an instance in specified AZ when vpc_subnet_id is not specified.
The current code does not consider the AZ and launches the instance in default subnet of default vpc for the specified region.
https://github.com/ansible-collections/amazon.aws/blob/main/plugins/modules/ec2_instance.py#L1584-L1593

Fixes ansible-collections#1120
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION


Without the fix, the below playbook will launch instance in default subnet ignoring specified AZ.
Example Playbook to launch instance in us-west-2b AZ.

---
- name: Spin up ec2 instance
  hosts: localhost
  gather_facts: false
  tasks:
    - name: Launch regular ec2 instances in us-west-1b
      amazon.aws.ec2_instance:
        name: "test-instance-us-west-1b"
        instance_type: t2.micro
        image_id: ami-xxxxx
        state: present
        availability_zone: us-west-1b
        region: us-west-1
        tags:
          terminate-this: yes
        network:
          assign_public_ip: yes
      register: create_result

    - ec2_instance_info:
        instance_ids:
          - "{{ create_result.instance_ids[0] }}"
        region: us-west-1
      register: info_result
    - assert:
        that: info_result.instances[0].placement.availability_zone == 'us-west-1b'

Reviewed-by: Mike Graves <[email protected]>
Reviewed-by: Gonéri Le Bouder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review feature This issue/PR relates to a feature request has_issue integration tests/integration mergeit Merge the PR (SoftwareFactory) module module needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants