New module - s3_website #662

Closed
wants to merge 2 commits into
from

Projects

None yet
@wimnat
Contributor
wimnat commented Jun 30, 2015

New module to manage an s3 bucket as a website

@bcoca
Member
bcoca commented Jul 3, 2015

@gregdek can you ping the amazon reviewers?

@gregdek
Contributor
gregdek commented Jul 5, 2015

@ansible/aws reviewers, please have a look

@gregdek gregdek added the new_plugin label Aug 31, 2015
@gregdek
Contributor
gregdek commented Sep 28, 2015

So let's try this again!

Adding new process. We will be evaluating all new module PRs according to this process, effective immediately.

Thanks for submitting this new module to Ansible Extras! This module is now in community review, a process that is open to all Ansible users. In order for this module to be approved, it must gain the following votes:

“works_for_me”: If you have tested the module thoroughly, including testing of all of the module’s options, and if the module works for you, please add “works_for_me” in the comments.

“passes_guidelines”: If you have gone through the module guidelines and the module meets all of the requirements, please add “passes_guidelines” in the comments. Guidelines are available here: http://docs.ansible.com/developing_modules.html#module-checklist

“needs_revision”: If the module fails to work for you, or if it doesn’t meet guidelines, please add “needs_revision” in the comments with details about what needs to be fixed.

When a module has both “works_for_me” and “passes_guidelines” tags, we will promote the module for inclusion in Ansible Extras. At this point, you will be expected to maintain the module by fixing bugs and evaluating pull requests in a timely manner.

Thanks again for submitting your Ansible module!


Ping @jsmartin @scicoin-project @tombamford @garethr @lorin @jarv @jsdalton @silviud @adq @zbal @zeekin @willthames @lwade @carsongee @defionscode @tastychutney @bpennypacker @loia -- although anyone can now review.

@robynbergeron robynbergeron removed the P3 label Sep 28, 2015
@willthames willthames and 1 other commented on an outdated diff Sep 28, 2015
cloud/amazon/s3_website.py
+
+# Remove website configuration from an s3 bucket
+- s3_website:
+ name: mybucket.com
+ state: absent
+
+# Configure an s3 bucket as a website with index and error pages
+- s3_website:
+ name: mybucket.com
+ suffix: home.htm
+ error_key: errors/404.htm
+ state: present
+
+'''
+
+import xml.etree.ElementTree as ET
@willthames
willthames Sep 28, 2015 Contributor

is this used?

@wimnat
wimnat Sep 28, 2015 Contributor

No, not any more. I removed the dependency. I'll remove this.

@njsaunders

Hi - Trying to use this but running in to issues:

An exception occurred during task execution. The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible-tmp-1457008606.98-273676758071704/s3_website", line 2430, in <module>
    main()
  File "/tmp/ansible-tmp-1457008606.98-273676758071704/s3_website", line 238, in main
    enable_bucket_as_website(connection, module)
  File "/tmp/ansible-tmp-1457008606.98-273676758071704/s3_website", line 116, in enable_bucket_as_website
    raise e
boto.exception.S3ResponseError: S3ResponseError: 400 Bad Request
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>InvalidArgument</Code><Message>RedirectAllRequestsTo cannot be provided in conjunction with other Routing Rules.</Message><ArgumentName>RedirectAllRequestsTo</ArgumentName><ArgumentValue>not null</ArgumentValue><RequestId>0000353A62F930ED</RequestId><HostId>OCu5tdfuH77ruNrvDpRT72s7ucZoIyjeE3nauwMJGIEaFnWHPG2eniXzRGMFLMGXXkrILB3QE10=</HostId></Error>

Using code:

    - name: Configure S3 bucket to redirect all requests to Fastly distribution
      s3_website:
        aws_access_key: "{{ assumed_role.sts_creds.access_key }}"
        aws_secret_key: "{{ assumed_role.sts_creds.secret_key }}"
        security_token: "{{ assumed_role.sts_creds.session_token }}"
        name: "{{s3_bucket_name}}"
        redirect_all_requests: "www.{{domain}}"
        state: present

Any advice gratefully received - I can't see find anything googling that error message?!

@manicminer manicminer commented on an outdated diff Mar 3, 2016
cloud/amazon/s3_website.py
+ error_key = module.params.get("error_key")
+ if error_key == "None":
+ error_key = None
+ redirect_all_requests = module.params.get("redirect_all_requests")
+ changed = False
+
+ if redirect_all_requests is not None:
+ redirect_location = RedirectLocation(hostname=redirect_all_requests)
+ else:
+ redirect_location = None
+
+ try:
+ bucket = connection.get_bucket(bucket_name)
+ if compare_bucket_as_website(bucket, module) is False:
+ bucket.delete_website_configuration()
+ bucket.configure_website(suffix, error_key, redirect_location)
@manicminer
manicminer Mar 3, 2016 Member

@njsaunders @wimnat Just tried this myself and it seems that the default value of index.html for the suffix argument causes the error. Adding suffix = None after line 104 fixes it for me.

@manicminer
Member

Also, I seriously thought I had already commented on this PR. It works_for_me, I am actively using this module locally.

@njsaunders

Excellent - Thanks @tombamford! Have instead changed the default to None:

suffix = dict(required=False, default=None),

Suggest this is fixed before it's merged.

@wimnat
Contributor
wimnat commented Mar 14, 2016

@tombamford @njsaunders - fixed up the issue. Appreciate a re-review

@njsaunders

LGTM :)

@wimnat
Contributor
wimnat commented Mar 14, 2016

@njsaunders cheers. Sorry to be pedantic but can you leave a works_for_me comment to appease the core team :)

@njsaunders

works_for_me

@defionscode
Member

missing RETURNS documentation

@wimnat
Contributor
wimnat commented Mar 20, 2016

@defionscode RETURN doc added

@pierreant-p

Would be really awesome to have this merged :)

@ahabman
ahabman commented May 2, 2016

works_for_me

@gregdek
Contributor
gregdek commented May 5, 2016

Thanks @wimnat for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@gregdek
Contributor
gregdek commented May 20, 2016

@wimnat A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

[This message brought to you by your friendly Ansibull-bot.]

@wimnat
Contributor
wimnat commented May 21, 2016

@njsaunders @manicminer @pierreant-p @defionscode

Hi all, i've rebuilt this module to use boto3 so it can get merged. I'd appreciate a retest using the new code and appropriate comment here if it's all good :)

@gregdek
Contributor
gregdek commented Jun 5, 2016

@wimnat Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request.

[This message brought to you by your friendly Ansibull-bot.]

@wimnat
Contributor
wimnat commented Jun 6, 2016

ready_for_review

@gregdek
Contributor
gregdek commented Jun 6, 2016

Thanks @wimnat for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion.

[This message brought to you by your friendly Ansibull-bot.]

@willthames willthames commented on an outdated diff Jun 6, 2016
cloud/amazon/s3_website.py
+
+def main():
+
+ argument_spec = ec2_argument_spec()
+ argument_spec.update(
+ dict(
+ name=dict(type='str', required=True),
+ state=dict(type='str', required=True, choices=['present', 'absent']),
+ suffix=dict(type='str', required=False, default='index.html'),
+ error_key=dict(type='str', required=False),
+ redirect_all_requests=dict(type='str', required=False)
+ )
+ )
+
+ module = AnsibleModule(argument_spec=argument_spec,
+ mutually_exclusive = [
@willthames
willthames Jun 6, 2016 Contributor

indentation here is not right - argument_spec probably wants to be on the line below aligned with mutually_exclusive - https://www.python.org/dev/peps/pep-0008/#indentation

@wimnat
Contributor
wimnat commented Jun 14, 2016

@willthames resolved

@ahabman
ahabman commented Jun 19, 2016

shipit

@wimnat
Contributor
wimnat commented Jul 24, 2016

@gregdek can you check the tags for this please? It says pending_action but i don't think that's correct. It's had a few shipits now and it builds ok so i'm not sure what the pending_action is for.

@gregdek gregdek removed the pending_action label Jul 25, 2016
@gregdek
Contributor
gregdek commented Jul 25, 2016

@wimnat yep, looks like it got pending_action in error. I'll add this one to the next new module review meeting.

@gregdek gregdek referenced this pull request in ansible/community Jul 25, 2016
Closed

Standing Agenda, New Modules Meeting #92

@nottwo
nottwo commented Jul 25, 2016

shipit

@alikins alikins commented on an outdated diff Aug 10, 2016
cloud/amazon/s3_website.py
+ try:
+ bucket_website.put(WebsiteConfiguration=_create_website_configuration(suffix, error_key, redirect_all_requests))
+ changed = True
+ except (ClientError, ParamValidationError) as e:
+ module.fail_json(msg=e.message, **camel_dict_to_snake_dict(e.response))
+ except KeyError as e:
+ try:
+ bucket_website.put(WebsiteConfiguration=_create_website_configuration(suffix, error_key, redirect_all_requests))
+ changed = True
+ except (ClientError, ParamValidationError) as e:
+ module.fail_json(msg=e.message, **camel_dict_to_snake_dict(e.response))
+ except ValueError as e:
+ module.fail_json(msg=str(e))
+
+ # Wait 5 secs before getting the website_config again to give it time to update
+ sleep(5)
@alikins
alikins Aug 10, 2016 Contributor

Use a regular import of system python modules instead of using the versions imported from basic.py:*
ie,

import time
<...>
                      time.sleep(5)
@ryansb
Member
ryansb commented Aug 12, 2016

@wimnat Looking at this PR, it would be right at home in the s3_bucket module, since it's configuration on an S3 bucket resource. I see you commented on a core/s3.py pull request saying that you wanted to prevent it from growing into an all-singing, all-dancing ec2_vpc module.

Would you please instead PR these features onto s3_bucket instead? I'll be happy to review & get them merged in.

@wimnat
Contributor
wimnat commented Aug 14, 2016

@ryansb I disagree that it fits in to the s3_bucket module for two reasons.

There are a number of parameters here related to how the s3 bucket is configured as a website which would add complexity to the s3_bucket module and in many cases users are unlikely to need them. Other functionality I included in s3_bucket module like versioning was much more straight-forward as its a simple on|off option. I am a firm believer in single responsibility principle

Secondly, this module is written with boto3 and s3_bucket is old boto so it's a more complicated task to add it in to s3_bucket.

@gregdek
Contributor
gregdek commented Aug 14, 2016

Thanks @wimnat for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@manicminer
Member

I'm using this module in production and I agree with @wimnat it should be separate from the s3_bucket module, both for reasons of complexity and simplicity of use. When configuring an S3 website either via the API or the AWS console, it's always a two step process of configuring the bucket, then configuring the website properties.

@ryansb
Member
ryansb commented Aug 19, 2016

Sounds like the community has spoken - thanks both of you

wimnat added some commits Jun 30, 2015
@wimnat @wimnat wimnat New module - s3_website 34f24d5
@wimnat wimnat Import time library rather than relying on Ansible basic.py
9bf9061
@wimnat
Contributor
wimnat commented Aug 20, 2016

ready_for_review

I think we have the appropriate number of shipits now

@gregdek
Contributor
gregdek commented Aug 20, 2016

Thanks @wimnat for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion.

[This message brought to you by your friendly Ansibull-bot.]

@ryansb
Member
ryansb commented Aug 22, 2016

Yup - there are sufficient shipits for inclusion.

@ryansb
Member
ryansb commented Aug 22, 2016

This has been squashed and committed as 119efdc

@ryansb ryansb closed this Aug 22, 2016
@wimnat wimnat deleted the wimnat:feature/s3_website branch Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment