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

ecs_ecr -- Return repository policy if it exists, even if we did not … #1171

Merged
merged 13 commits into from
Jun 4, 2022

Conversation

karcadia
Copy link
Contributor

@karcadia karcadia commented May 26, 2022

Return repository policy if it exists, even if we did not create or modify it.

SUMMARY

Existing behavior will only print the ECR repo (permissions) policy if we just created/edited that policy. There is no way to just pull the existing policy and use it in a subsequent task.

New behavior will print the existing ECR repo policy information if it exists, even if we did not just create/edit that policy.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ecs_ecr -- run function

ADDITIONAL INFORMATION

Given the scenario that my ECR repo already exists and already has a permissions policy on it. I would like to use ansible to retrieve that policy.
Today this module will only return the policy as part of the return object if we are telling the module to create or edit the policy.
After merging, the module will create the policy if it was defined, or update the policy if needed, and if neither of those were defined (else) we will check for an existing permissions policy and add it to the return object.

Given the example tasks:
- name: Pull the existing permissions policy for the ECR repo.
      community.aws.ecs_ecr:
        region: "{{ AWS_REGION }}"
        aws_access_key: "{{ survey_access_key_id }}"
        aws_secret_key: "{{ survey_secret_access_key }}"
        aws_security_token: "{{ survey_session_token }}"
        validate_certs: False
        name: 'example/nginx'
      register: ecr_repo_info

    - name: debug
      debug:
        msg: "{{ ecr_repo_info }}"

Here is the current behavior:
ASK [debug] *********************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": {
        "changed": false,
        "created": false,
        "failed": false,
        "repository": {
            "createdAt": "2022-05-19T11:15:49-05:00",
            "encryptionConfiguration": {
                "encryptionType": "AES256"
            },
            "imageScanningConfiguration": {
                "scanOnPush": false
            },
            "imageTagMutability": "MUTABLE",
            "registryId": "12345",
            "repositoryArn": "arn:aws:ecr:us-east-1:123456789:repository/example/nginx",
            "repositoryName": "example/nginx",
            "repositoryUri": "123456789.dkr.ecr.us-east-1.amazonaws.com/example/nginx"
        },
        "state": "present"
    }
}

And here is the post-merge output:
ASK [debug] *********************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": {
        "changed": false,
        "created": false,
        "failed": false,
        "policy": {
            "Statement": [
                {
                    "Action": [
                        "ecr:GetDownloadUrlForLayer",
                        "ecr:BatchGetImage",
                        "ecr:BatchCheckLayerAvailability",
                        "ecr:ListImages",
                        "ecr:DescribeImages",
                        "ecr:DescribeRepositories"
                    ],
                    "Effect": "Allow",
                    "Principal": {
                        "AWS": "arn:aws:iam::12345678:root"
                    },
                    "Sid": "Allow managed accounts to access this repo."
                }
            ],
            "Version": "2008-10-17"
        },
        "repository": {
            "createdAt": "2022-05-19T11:15:49-05:00",
            "encryptionConfiguration": {
                "encryptionType": "AES256"
            },
            "imageScanningConfiguration": {
                "scanOnPush": false
            },
            "imageTagMutability": "MUTABLE",
            "registryId": "12345",
            "repositoryArn": "arn:aws:ecr:us-east-1:123456789:repository/example/nginx",
            "repositoryName": "example/nginx",
            "repositoryUri": "123456789.dkr.ecr.us-east-1.amazonaws.com/example/nginx"
        },
        "state": "present"
    }
}

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels May 26, 2022
@github-actions
Copy link

github-actions bot commented May 26, 2022

Docs Build 📝

Thank you for contribution!✨

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

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 24s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 59s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 02s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 22s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 25s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 11s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 53s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 47s
✔️ ansible-test-splitter SUCCESS in 2m 34s
✔️ integration-community.aws-1 SUCCESS in 5m 20s
⚠️ 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

Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

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

I'm okey with that change. An ecs_ecr_info module would be also helpful in the future.

If an iam policy should also be transformed to snake_dict, must be discussed first.

Otherwise it LGTM. Just a changelog fragment is missing.

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels May 27, 2022
@ansibullbot
Copy link

@karcadia this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 27, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

ansible-galaxy-importer FAILURE in 3m 41s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 47s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 20s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 21s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 14m 33s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 04s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 55s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 27s
✔️ ansible-test-splitter SUCCESS in 2m 29s
integration-community.aws-1 RETRY_LIMIT in 1m 30s
⚠️ 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

@ansibullbot
Copy link

@karcadia this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added community_review and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels May 27, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 19s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 42s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 02s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 33s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 13s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 04s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 30s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 16s
✔️ ansible-test-splitter SUCCESS in 2m 24s
✔️ integration-community.aws-1 SUCCESS in 5m 46s
⚠️ 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

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels May 27, 2022
Copy link
Contributor Author

@karcadia karcadia left a comment

Choose a reason for hiding this comment

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

Trying to clear the needs_revision tag.

The changelog fragment has been added.
I accepted the change for the added in version 4.0.
There was a discussion around casing but I didn't see a specific action item so should be ready for review. Thanks.

@karcadia karcadia requested a review from markuman May 27, 2022 18:33
@ansibullbot ansibullbot added community_review and removed new_contributor Help guide this first time contributor labels Jun 2, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 19s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 16s
✔️ ansible-test-sanity-docker-devel SUCCESS in 13m 53s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 53s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 08s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 44s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 14s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 12m 03s
✔️ ansible-test-splitter SUCCESS in 2m 46s
✔️ integration-community.aws-1 SUCCESS in 5m 08s
⚠️ 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

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Jun 2, 2022
@markuman markuman requested a review from alinabuzachis June 3, 2022 09:08
@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 3, 2022
Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@karcadia Thank you. LGTM!

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 5m 56s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 10s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 49s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 07s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 04s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 58s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 20s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 46s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 47s
✔️ ansible-test-splitter SUCCESS in 2m 33s
✔️ integration-community.aws-1 SUCCESS in 5m 50s
⚠️ 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

@markuman markuman added the mergeit Merge the PR (SoftwareFactory) label Jun 4, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 4m 00s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 27s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 29s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 54s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 57s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 12m 32s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 30s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 38s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 05s
✔️ ansible-test-splitter SUCCESS in 2m 49s
✔️ integration-community.aws-1 SUCCESS in 5m 11s
⚠️ 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 24c3df8 into ansible-collections:main Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request integration tests/integration mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants