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

fix aws_s3 module to use custum s3_url. #36832

Merged
merged 2 commits into from
Jun 7, 2018

Conversation

cgroschupp
Copy link
Contributor

@cgroschupp cgroschupp commented Feb 28, 2018

SUMMARY

It is not possible to use aws_s3 modules with a customized s3_url.

With a custom s3_url url it is never possible to reach the else in get_s3_connection.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

aws_s3

ANSIBLE VERSION
2.4.3.0
ADDITIONAL INFORMATION

The walrus code extracts the hostname from the s3_url and boto3 throws Invalid Endpoint

  - aws_s3:
      bucket: hermestest123
      aws_access_key: XXXXX
      aws_secret_key: XXXXXXXXXXXX
      s3_url: https://obs.otc.t-systems.com
      mode: create

The boto3 documentation says that you need to define a full URL with a schema.

http://boto3.readthedocs.io/en/latest/reference/core/session.html

endpoint_url (string) -- The complete URL to use for the constructed client. Normally, botocore 
will automatically construct the appropriate URL to use when communicating with a service. You 
can specify a complete URL (including the "http/https" scheme) to override this behavior. 
If this value is provided, then use_ssl is ignored.
The full traceback is:
  File "/tmp/ansible_YwcUE7/ansible_modlib.zip/ansible/module_utils/ec2.py", line 99, in boto3_conn
    return _boto3_conn(conn_type=conn_type, resource=resource, region=region, endpoint=endpoint, **params)
  File "/tmp/ansible_YwcUE7/ansible_modlib.zip/ansible/module_utils/ec2.py", line 118, in _boto3_conn
    client = boto3.session.Session(profile_name=profile).client(resource, region_name=region, endpoint_url=endpoint, **params)
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/boto3/session.py", line 263, in client
    aws_session_token=aws_session_token, config=config)
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/botocore/session.py", line 861, in create_client
    client_config=config, api_version=api_version)
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/botocore/client.py", line 76, in create_client
    verify, credentials, scoped_config, client_config, endpoint_bridge)
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/botocore/client.py", line 285, in _get_client_args
    verify, credentials, scoped_config, client_config, endpoint_bridge)
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/botocore/args.py", line 79, in get_client_args
    timeout=(new_config.connect_timeout, new_config.read_timeout))
  File "/home/groschuppchr/venv/lib/python2.7/site-packages/botocore/endpoint.py", line 289, in create_endpoint
    raise ValueError("Invalid endpoint: %s" % endpoint_url)

fatal: [localhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "aws_access_key": "S3RHEYTMWRHJ6QW0C7ZA",
            "aws_secret_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "bucket": "hermestest123",
            "dest": null,
            "ec2_url": null,
            "encrypt": true,
            "expiry": 600,
            "headers": null,
            "ignore_nonexistent_bucket": false,
            "marker": "",
            "max_keys": 1000,
            "metadata": null,
            "mode": "create",
            "object": null,
            "overwrite": "always",
            "permission": [
                "private"
            ],
            "prefix": "",
            "profile": null,
            "region": null,
            "retries": 0,
            "rgw": false,
            "s3_url": "https://obs.otc.t-systems.com",
            "security_token": null,
            "src": null,
            "validate_certs": false,
            "version": null
        }
    },
    "msg": "There is an issue in the code of the module. You must specify either both, resource or client to the conn_type parameter in the boto3_conn function call"
}

With an s3_url without schema, the endpoint is always empty and boto connects to the aws endpoints.

  - aws_s3:
      bucket: hermestest123
      aws_access_key: XXXXX
      aws_secret_key: XXXXXXXXXXXX
      s3_url: obs.otc.t-systems.com
      mode: create
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ClientError: An error occurred (403) when calling the HeadBucket operation: Forbidden
fatal: [localhost]: FAILED! => {"changed": false, "error": {"code": "403", "message": "Forbidden"}, "msg": "Failed while looking up bucket (during bucket_check) hermestest123.", "response_metadata": {"host_id": "B65bpv5BmRUQ1xgEzvXc3+v1syjrl08tNAWCYGl8H489uADMJz05OQMbQwUcAvzVikVvjJgTtTU=", "http_headers": {"content-type": "application/xml", "date": "Wed, 28 Feb 2018 08:27:19 GMT", "server": "AmazonS3", "transfer-encoding": "chunked", "x-amz-id-2": "B65bpv5BmRUQ1xgEzvXc3+v1syjrl08tNAWCYGl8H489uADMJz05OQMbQwUcAvzVikVvjJgTtTU=", "x-amz-request-id": "C4A401E2DFDA9DB4"}, "http_status_code": 403, "request_id": "C4A401E2DFDA9DB4", "retry_attempts": 0}}

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

@ansibot ansibot added aws bugfix_pull_request cloud core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 28, 2018
@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label Feb 28, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@k0ste
Copy link
Contributor

k0ste commented Mar 6, 2018

You should set rgw: true for fakeS3 usage.
But better behavior I think is if s3_url is present - use it without additional options.

@roadmapper
Copy link
Contributor

I would agree that that statement @k0ste. I don't seem to see any parameters or settings that are specific to Ceph, Walrus, fakes3, Eucalyptus in this module (the HTTPS/HTTP settings should be applied to any s3_url passed in). @s-hertel, would it be ok to change the logic for s3_url so that if specified use that and deprecate the rgw flag?

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 10, 2018
@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 21, 2018
@ansibot ansibot added the traceback This issue/PR includes a traceback. label May 29, 2018
@s-hertel
Copy link
Contributor

@roadmapper Yeah, I agree that it would be ideal to deprecate the rgw flag and treat everything the same (besides perhaps populating some sane defaults like is happening for fakes3) but I'm hesitant since there is no one reliably working on this who can test Walrus/Ceph/fakes3.

Copy link
Contributor

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

@cgroschupp This code can't just be removed in case anyone has been relying on this for walrus (the module is marked as stableinterface) so it would need a deprecation warning period at the least.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 29, 2018
@s-hertel s-hertel dismissed their stale review May 29, 2018 20:01

I've thought some more about this and this approach actually seems fine, though it's unclear if this will break or fix Walrus. In theory Walrus is a drop-in for S3 and should work like this. If anyone uses Walrus and can test this, that would be great. I may try to set it up to test but doubt I will be able to on OS X.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 29, 2018
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jun 6, 2018
@s-hertel s-hertel force-pushed the feature_fix_custom_s3_endpoint branch from 6300294 to 8b461b4 Compare June 7, 2018 17:40
Copy link
Contributor

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

I rebased this due to conflicts. I haven't been able to test walrus but have been using this with Minio which also has an S3-compatible API.

@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jun 7, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 7, 2018
@ryansb ryansb merged commit e59742e into ansible:devel Jun 7, 2018
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
@ansible ansible locked and limited conversation to collaborators Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 aws bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. traceback This issue/PR includes a traceback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants