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

s3_object - Be more defensive when checking the results of get_bucket_ownership_controls #1117

Merged

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Oct 5, 2022

SUMMARY

Scaleway have half-implemented get_bucket_ownership_controls, but don't return any rules. Be a little more defensive when checking the return value of get_bucket_ownership_controls. The API doesn't strictly say a Rule will always be returned.

Fixes: #1115

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

s3_object

ADDITIONAL INFORMATION

@tremble tremble requested a review from mandar242 October 5, 2022 12:41
@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage owner_pr PR created by owner/maintainer plugins plugin (any type) labels Oct 5, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 5m 39s
✔️ build-ansible-collection SUCCESS in 6m 23s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 00s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 45s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 41s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 8m 03s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 58s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 58s
✔️ cloud-tox-py3 SUCCESS in 4m 24s
✔️ ansible-test-splitter SUCCESS in 3m 56s
✔️ integration-amazon.aws-1 SUCCESS in 10m 38s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ 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
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 3m 50s

@tremble tremble requested a review from alinabuzachis October 5, 2022 14:40
@tremble tremble added mergeit Merge the PR (SoftwareFactory) backport-5 PR should be backported to the stable-5 branch labels Oct 5, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline). For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

✔️ ansible-galaxy-importer SUCCESS in 3m 50s
✔️ build-ansible-collection SUCCESS in 6m 43s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 8m 23s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 27s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 16s
ansible-test-units-amazon-aws-python36 POST_FAILURE in 7m 28s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 30s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 02s
✔️ cloud-tox-py3 SUCCESS in 3m 03s
✔️ ansible-test-splitter SUCCESS in 2m 49s
✔️ integration-amazon.aws-1 SUCCESS in 15m 40s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
✔️ integration-community.aws-1 SUCCESS in 38m 26s
✔️ integration-community.aws-2 SUCCESS in 26m 13s
✔️ integration-community.aws-3 SUCCESS in 6m 58s
✔️ integration-community.aws-4 SUCCESS in 43m 58s
✔️ integration-community.aws-5 SUCCESS in 31m 35s
✔️ integration-community.aws-6 SUCCESS in 32m 27s
✔️ integration-community.aws-7 SUCCESS in 46m 41s
✔️ integration-community.aws-8 SUCCESS in 37m 35s
✔️ integration-community.aws-9 SUCCESS in 22m 51s
✔️ integration-community.aws-10 SUCCESS in 22m 40s
✔️ integration-community.aws-11 SUCCESS in 12m 02s
✔️ integration-community.aws-12 SUCCESS in 41m 14s
✔️ integration-community.aws-13 SUCCESS in 7m 38s
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 2m 31s

@tremble
Copy link
Contributor Author

tremble commented Oct 5, 2022

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 3m 40s
✔️ build-ansible-collection SUCCESS in 5m 05s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 43s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 19s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 01s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 28s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 7m 21s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 26s
✔️ cloud-tox-py3 SUCCESS in 2m 47s
✔️ ansible-test-splitter SUCCESS in 2m 46s
✔️ integration-amazon.aws-1 SUCCESS in 9m 58s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
✔️ integration-community.aws-1 SUCCESS in 31m 17s
✔️ integration-community.aws-2 SUCCESS in 26m 31s
✔️ integration-community.aws-3 SUCCESS in 5m 13s
✔️ integration-community.aws-4 SUCCESS in 35m 20s
✔️ integration-community.aws-5 SUCCESS in 29m 23s
✔️ integration-community.aws-6 SUCCESS in 35m 17s
✔️ integration-community.aws-7 SUCCESS in 45m 56s
✔️ integration-community.aws-8 SUCCESS in 39m 11s
✔️ integration-community.aws-9 SUCCESS in 22m 13s
✔️ integration-community.aws-10 SUCCESS in 28m 58s
✔️ integration-community.aws-11 SUCCESS in 12m 31s
✔️ integration-community.aws-12 SUCCESS in 49m 58s
✔️ integration-community.aws-13 SUCCESS in 8m 05s
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 2m 12s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 0c3239b into ansible-collections:main Oct 5, 2022
@patchback
Copy link

patchback bot commented Oct 5, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/0c3239be7591b722ef4c76e8cf4ad40a217463d7/pr-1117

Backported as #1122

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Oct 5, 2022
…_ownership_controls (#1117)

s3_object - Be more defensive when checking the results of get_bucket_ownership_controls

SUMMARY
Scaleway have half-implemented get_bucket_ownership_controls, but don't return any rules.  Be a little more defensive when checking the return value of get_bucket_ownership_controls.  The API doesn't strictly say a Rule will always be returned.
Fixes: #1115
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
s3_object
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 0c3239b)
tremble added a commit that referenced this pull request Oct 6, 2022
…_ownership_controls (#1117)

s3_object - Be more defensive when checking the results of get_bucket_ownership_controls

SUMMARY
Scaleway have half-implemented get_bucket_ownership_controls, but don't return any rules.  Be a little more defensive when checking the return value of get_bucket_ownership_controls.  The API doesn't strictly say a Rule will always be returned.
Fixes: #1115
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
s3_object

ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 0c3239b)
tremble pushed a commit that referenced this pull request Oct 6, 2022
…_ownership_controls (#1117) (#1122)

SUMMARY

Scaleway have half-implemented get_bucket_ownership_controls, but don't return any rules.  Be a little more defensive when checking the return value of get_bucket_ownership_controls.  The API doesn't strictly say a Rule will always be returned.

Fixes: #1115

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

s3_object

ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 0c3239b)
saito-hideki pushed a commit to saito-hideki/amazon.aws that referenced this pull request Oct 18, 2022
)

route53: add support for GeoLocation parameter

SUMMARY

Added support for GeoLocation parameter to community.aws.route53
Fixes ansible-collections#89.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

route53
ADDITIONAL INFORMATION

Uses https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/route53.html#Route53.Client.change_resource_record_sets

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Sloane Hertel <None>
Reviewed-by: Alina Buzachis <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@29e51f9
saito-hideki pushed a commit to saito-hideki/amazon.aws that referenced this pull request Oct 18, 2022
…ctions#1379)

route53: Restore support for zero weighted DNS records

SUMMARY
In ansible-collections#1117 (comment) and https://github.com/ansible-collections/community.aws/pull/1117/files#r869391659 this line was recommended to be simplified, but not any will also return true if weight_in has a value of 0, not only when it is None
Fixes ansible-collections#1378
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53
ADDITIONAL INFORMATION

Previously it was possible to create weighted records with a weight of 0. Currently the playbook below returns the error:
You have specified identifier which makes sense only if you specify one of: weight, region, geo_location or failover.

- name: Bug demo
  hosts: localhost
  tasks:
    - name: Set 0 weight for old env
      route53:
        wait: yes
        ttl: '5'
        type: 'CNAME'
        identifier: old
        overwrite: yes
        record: 'record.example.com.'
        zone: 'example.com.'
        value: 'record-old.example.com.'
        weight: '0'
        state: present

Reviewed-by: Mark Chappell <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@9195021
@tremble tremble deleted the issue/1115 branch October 21, 2022 07:34
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
)

route53: add support for GeoLocation parameter

SUMMARY

Added support for GeoLocation parameter to community.aws.route53
Fixes ansible-collections#89.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

route53
ADDITIONAL INFORMATION


Uses https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/route53.html#Route53.Client.change_resource_record_sets

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Sloane Hertel <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…ctions#1379)

route53: Restore support for zero weighted DNS records

SUMMARY
In ansible-collections#1117 (comment) and https://github.com/ansible-collections/community.aws/pull/1117/files#r869391659 this line was recommended to be simplified, but not any will also return true if weight_in has a value of 0, not only when it is None
Fixes ansible-collections#1378
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
route53
ADDITIONAL INFORMATION


Previously it was possible to create weighted records with a weight of 0. Currently the playbook below returns the error:
You have specified identifier which makes sense only if you specify one of: weight, region, geo_location or failover.


- name: Bug demo
  hosts: localhost
  tasks:
    - name: Set 0 weight for old env
      route53:
        wait: yes
        ttl: '5'
        type: 'CNAME'
        identifier: old
        overwrite: yes
        record: 'record.example.com.'
        zone: 'example.com.'
        value: 'record-old.example.com.'
        weight: '0'
        state: present

Reviewed-by: Mark Chappell <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
)

route53: add support for GeoLocation parameter

SUMMARY

Added support for GeoLocation parameter to community.aws.route53
Fixes ansible-collections#89.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

route53
ADDITIONAL INFORMATION


Uses https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/route53.html#Route53.Client.change_resource_record_sets

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Sloane Hertel <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…ctions#1379)

route53: Restore support for zero weighted DNS records

SUMMARY
In ansible-collections#1117 (comment) and https://github.com/ansible-collections/community.aws/pull/1117/files#r869391659 this line was recommended to be simplified, but not any will also return true if weight_in has a value of 0, not only when it is None
Fixes ansible-collections#1378
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
route53
ADDITIONAL INFORMATION


Previously it was possible to create weighted records with a weight of 0. Currently the playbook below returns the error:
You have specified identifier which makes sense only if you specify one of: weight, region, geo_location or failover.


- name: Bug demo
  hosts: localhost
  tasks:
    - name: Set 0 weight for old env
      route53:
        wait: yes
        ttl: '5'
        type: 'CNAME'
        identifier: old
        overwrite: yes
        record: 'record.example.com.'
        zone: 'example.com.'
        value: 'record-old.example.com.'
        weight: '0'
        state: present

Reviewed-by: Mark Chappell <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
)

route53: add support for GeoLocation parameter

SUMMARY

Added support for GeoLocation parameter to community.aws.route53
Fixes ansible-collections#89.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

route53
ADDITIONAL INFORMATION


Uses https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/route53.html#Route53.Client.change_resource_record_sets

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Sloane Hertel <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…ctions#1379)

route53: Restore support for zero weighted DNS records

SUMMARY
In ansible-collections#1117 (comment) and https://github.com/ansible-collections/community.aws/pull/1117/files#r869391659 this line was recommended to be simplified, but not any will also return true if weight_in has a value of 0, not only when it is None
Fixes ansible-collections#1378
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
route53
ADDITIONAL INFORMATION


Previously it was possible to create weighted records with a weight of 0. Currently the playbook below returns the error:
You have specified identifier which makes sense only if you specify one of: weight, region, geo_location or failover.


- name: Bug demo
  hosts: localhost
  tasks:
    - name: Set 0 weight for old env
      route53:
        wait: yes
        ttl: '5'
        type: 'CNAME'
        identifier: old
        overwrite: yes
        record: 'record.example.com.'
        zone: 'example.com.'
        value: 'record-old.example.com.'
        weight: '0'
        state: present

Reviewed-by: Mark Chappell <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-5 PR should be backported to the stable-5 branch bug This issue/PR relates to a bug community_review mergeit Merge the PR (SoftwareFactory) module module needs_triage owner_pr PR created by owner/maintainer plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.0.0 : aws_s3 may fail on some S3 compatible provider that do not implement OwnershipControls
3 participants