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

backup_selection conditions update incorrectly reports changed #1613

Closed
1 task done
hakbailey opened this issue Jun 13, 2023 · 1 comment · Fixed by #1633
Closed
1 task done

backup_selection conditions update incorrectly reports changed #1613

hakbailey opened this issue Jun 13, 2023 · 1 comment · Fixed by #1633
Assignees
Labels
Milestone

Comments

@hakbailey
Copy link
Contributor

Summary

When adding the conditions option to an idempotent backup selection update, the selection does not change but the result returns "changed": true. This happens because a user can pass one or more of the four possible conditions suboptions, and any suboptions not passed will simply not exist in the module data (rather than being present with an empty or None value).

Later, when we compare the provided conditions with the conditions value from the current AWS backup selection, they are never equal because the boto3 client's get_backup_selection() returns all condition suboptions instead of just the ones that have values. Any that do not have values are returned as empty lists, which are not present in the data provided by the user.

Ultimately this still results in the backup selection update being correct, it's just the returned changed value that's wrong.

Issue Type

Bug Report

Component Name

backup_selection

Ansible Version

$ ansible --version
ansible [core 2.15.0]
  config file = /Users/hebailey/.ansible.cfg
  configured module search path = ['/Users/hebailey/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/hebailey/.pyenv/versions/3.11.3/envs/ansible-core-2.15/lib/python3.11/site-packages/ansible
  ansible collection location = /Users/hebailey:/Users/hebailey
  executable location = /Users/hebailey/.pyenv/versions/ansible-core-2.15/bin/ansible
  python version = 3.11.3 (main, May 15 2023, 16:53:55) [Clang 14.0.3 (clang-1403.0.22.14.1)] (/Users/hebailey/.pyenv/versions/3.11.3/envs/ansible-core-2.15/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True

Collection Versions

$ ansible-galaxy collection list
Collection       Version   
---------------- ----------
.mypy_cache.3.11 *         
amazon.aws       7.0.0-dev0
cloud.aws_ops    1.0.3     
cloud.terraform  1.1.0     
community.aws    6.0.0-dev0

AWS SDK versions

$ pip show boto boto3 botocore
Name: boto3
Version: 1.26.151
Summary: The AWS SDK for Python
Home-page: https://github.com/boto/boto3
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /Users/hebailey/.pyenv/versions/3.11.3/envs/ansible-core-2.15/lib/python3.11/site-packages
Requires: botocore, jmespath, s3transfer
Required-by: 
---
Name: botocore
Version: 1.29.151
Summary: Low-level, data-driven core of boto 3.
Home-page: https://github.com/boto/botocore
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /Users/hebailey/.pyenv/versions/3.11.3/envs/ansible-core-2.15/lib/python3.11/site-packages
Requires: jmespath, python-dateutil, urllib3
Required-by: boto3, s3transfer

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

This is modified from the existing integration test to include conditions.

    - name: Create an AWS Backup selection
      amazon.aws.backup_selection:
        selection_name: "{{ backup_selection_name }}"
        backup_plan_name: "{{ backup_plan_name }}"
        iam_role_arn: "{{ iam_role.iam_role.arn }}"
        list_of_tags:
          - condition_type: "STRINGEQUALS"
            condition_key: "backup"
            condition_value: "daily"
      register: _create_result_backup_selection

# Skipping several tests irrelevant to this issue

    - name: Modify an AWS Backup selection
      amazon.aws.backup_selection:
        selection_name: "{{ backup_selection_name }}"
        backup_plan_name: "{{ backup_plan_name }}"
        iam_role_arn: "{{ iam_role.iam_role.arn }}"
        list_of_tags:
          - condition_type: "STRINGEQUALS"
            condition_key: "backup"
            condition_value: "weekly"
        conditions:
          string_not_equals:
            - condition_key: "aws:ResourceTag/env"
              condition_value: "test"
      register: _modify_result_backup_selection

    - name: Verify modify result
      ansible.builtin.assert:
        that:
          - _modify_result_backup_selection.changed

   - name: Modify an AWS Backup selection (idempotency)
      amazon.aws.backup_selection:
        selection_name: "{{ backup_selection_name }}"
        backup_plan_name: "{{ backup_plan_name }}"
        iam_role_arn: "{{ iam_role.iam_role.arn }}"
        list_of_tags:
          - condition_type: "STRINGEQUALS"
            condition_key: "backup"
            condition_value: "weekly"
        conditions:
          string_not_equals:
            - condition_key: "aws:ResourceTag/env"
              condition_value: "test"
      register: _modify_result_backup_selection

    - name: Verify modify (idempotency) result
      ansible.builtin.assert:
        that:
          - not _modify_result_backup_selection.changed

Expected Results

TASK [backup_selection : Create an AWS Backup selection] ***********************
changed: [testhost]

TASK [backup_selection : Modify an AWS Backup selection] ***********************
changed: [testhost]

TASK [backup_selection : Verify modify result] ****************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [backup_selection : Modify an AWS Backup selection (idempotency)] *********
changed: [testhost]

TASK [backup_selection : Verify modify (idempotency) result] ****************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

Actual Results

TASK [backup_selection : Create an AWS Backup selection] ***********************
changed: [testhost]

TASK [backup_selection : Modify an AWS Backup selection] ***********************
changed: [testhost]

TASK [backup_selection : Verify modify result] ****************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [backup_selection : Modify an AWS Backup selection (idempotency)] *********
changed: [testhost]

TASK [backup_selection : Verify modify (idempotent) result] ****************************************
fatal: [testhost]: FAILED! => {
    "assertion": "not _modify_result_backup_selection.changed",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@hakbailey hakbailey self-assigned this Jun 13, 2023
@hakbailey
Copy link
Contributor Author

I intend to submit a PR for this but because describing the issue is kind of involved, I wanted to document it first rather than trying to write all the details up in the PR summary.

@hakbailey hakbailey added this to the 6.2.0 milestone Jun 22, 2023
softwarefactory-project-zuul bot pushed a commit that referenced this issue Jul 4, 2023
Document and validate backup_selection conditions suboptions

SUMMARY
Adds documentation and validation for all conditions suboptions in backup_selection module. Fixes #1613
Additionally fixes a bug in module_utils.backup that caused an empty list to be returned from get_selection_details() when multiple backup selections exist for a given backup plan.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
backup_selection
module_utils.backup
ADDITIONAL INFORMATION
See #1613 for detailed description of related issue.

Reviewed-by: Jill R
Reviewed-by: Alina Buzachis
patchback bot pushed a commit that referenced this issue Jul 4, 2023
Document and validate backup_selection conditions suboptions

SUMMARY
Adds documentation and validation for all conditions suboptions in backup_selection module. Fixes #1613
Additionally fixes a bug in module_utils.backup that caused an empty list to be returned from get_selection_details() when multiple backup selections exist for a given backup plan.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
backup_selection
module_utils.backup
ADDITIONAL INFORMATION
See #1613 for detailed description of related issue.

Reviewed-by: Jill R
Reviewed-by: Alina Buzachis
(cherry picked from commit cfeffe6)
softwarefactory-project-zuul bot pushed a commit that referenced this issue Jul 4, 2023
…1638)

[PR #1633/cfeffe66 backport][stable-6] Document and validate backup_selection conditions suboptions

This is a backport of PR #1633 as merged into main (cfeffe6).
SUMMARY
Adds documentation and validation for all conditions suboptions in backup_selection module. Fixes #1613
Additionally fixes a bug in module_utils.backup that caused an empty list to be returned from get_selection_details() when multiple backup selections exist for a given backup plan.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
backup_selection
module_utils.backup
ADDITIONAL INFORMATION
See #1613 for detailed description of related issue.

Reviewed-by: Alina Buzachis
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
…tions#1613)

Fixup opensearch when using advanced security options

Fix ansible-collections#1560

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
…tions#1613)

Fixup opensearch when using advanced security options

Fix ansible-collections#1560

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 24, 2023
…tions#1613)

Fixup opensearch when using advanced security options

Fix ansible-collections#1560

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant