boto_s3_bucket.present is not idempotent

[zerthimon]

Description of Issue/Question

boto_s3_bucket.present falsely detects changes in s3 bucket permissions and applies the state eventhough there are no changes to be made.

----------
          ID: backup_bucket
    Function: boto_s3_bucket.present
        Name: server-backup-bucket
      Result: True
     Comment: S3 bucket server-backup-bucket is present.
     Started: 18:56:08.666017
    Duration: 629.986 ms
     Changes:   
              ----------
              new:
                  ----------
                  ACL:
                      ----------
                      ACL:
                          private
              old:
                  ----------
                  ACL:
                      ----------
                      Grants:
                          |_
                            ----------
                            Grantee:
                                ----------
                                ID:
                                    xxx
                                Type:
                                    CanonicalUser
                            Permission:
                                FULL_CONTROL
                      Owner:
                          ----------
                          ID:
                              xxx

Setup

backup_bucket:
  boto_s3_bucket.present:
    - name: server-backup-bucket
    - Bucket: server-backup-bucket
    - LocationConstraint: eu-central-1
    - region: eu-central-1

Steps to Reproduce Issue

Run SLS a few times, and see that there are the same changes every run.

Versions Report

Salt Version:
           Salt: 2016.3.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.2.2
       dateutil: 1.5
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.3.0
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.4

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.19.0-59-generic
         system: Linux
        version: Ubuntu 14.04 trusty

[Ch3LL]

@zerthimon I am able to replicate this. Looks like there needs to be some logic added to ensure its not run again if its already present. Here is my output as well:

              new:
                  ----------
                  ACL:
                      ----------
                      ACL:
                          private
              old:
                  ----------
                  ACL:
                      ----------
                      Grants:
                          |_
                            ----------
                            Grantee:
                                ----------
                                ID:
                                    <id>
                                Type:
                                    CanonicalUser
                            Permission:
                                FULL_CONTROL
                      Owner:
                          ----------
                          ID:
                              <id>

[danslimmon]

At least in my case, this seems to be due to an ACL mismatch.

When states.boto_s3_bucket.present creates a new bucket, it calls modules.boto_s3_bucket.create, which passes ACL=None as a default argument to the AWS API. The next time present is called and it retrieves that newly-created bucket's ACL, it wants to compare the bucket's ACL with a default argument of {'ACL': 'private'}. But the thing it gets back from the API to compare against is much more involved:

{
    u'Grants': [{
        u'Grantee': {
            u'DisplayName': '<my username>',
            u'ID': '<long hex string>',
            u'Type': 'CanonicalUser'
        },
        u'Permission': 'FULL_CONTROL'
    }],
    u'Owner': {
        u'DisplayName': '<my username>',
        u'ID': '<long hex string>'
    }
}

[zerthimon]

I've tried the state with different ACL confgs (to have the same parameters that api returns for the ACL), but couldn't make it run just once.

[danslimmon]

@zerthimon & @Ch3LL: My PR #33776 resolved this issue for me. I'm curious whether it works for you two as well.

[zerthimon]

@danslimmon Works well for me!

----------
          ID: backup_bucket
    Function: boto_s3_bucket.present
        Name: server-backup
      Result: True
     Comment: S3 server-backup is present.
     Started: 19:10:01.493536
    Duration: 380.31 ms
     Changes: