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

acme module fails to set file permissions if the certificate is already present #48626

Closed
nbraud opened this Issue Jul 17, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@nbraud
Contributor

nbraud commented Jul 17, 2018

Description of Issue/Question

Setup

somegroup:
  group.present

some.example.com:
  acme.cert:
    - email: webmaster@example.com
    - group: somegroup

Steps to Reproduce Issue

  1. Run the above state file without the group: somegroup parameter.
  2. Once the certificate is acquired (successful highstate), add the group parameter.
  3. Highstate the minion. The module reports nothing changed, and the private key file is indeed still owned by group root.

Versions Report

2018.3.2, but looking at the code suggests that it's still present in develop.

$ salt --versions-report
Salt Version:
           Salt: 2018.3.2
 
Dependency Versions:
           cffi: 1.10.0
       cherrypy: 3.2.2
       dateutil: 2.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.14 (default, May  2 2018, 18:31:34)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.5.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
 
System Versions:
           dist:   
         locale: UTF-8
        machine: x86_64
        release: 4.14.51-60.38.amzn1.x86_64
         system: Linux
        version: Not Installed
@astronouth7303

This comment has been minimized.

Contributor

astronouth7303 commented Jul 17, 2018

This seems to happen because of https://github.com/saltstack/salt/blob/v2018.3.2/salt/modules/acme.py#L172-L175 where it just exits if acme does nothing instead of checking the group.

nbraud added a commit to nbraud/salt that referenced this issue Jul 17, 2018

module/acme: Do not exit early when the certificate already exists
Otherwise, acme.cert might fail to cert permissions on the certificate.

Close saltstack#48626

@nbraud nbraud referenced this issue Jul 17, 2018

Merged

Bug fixes in the acme module & state #48635

3 of 3 tasks complete

nbraud added a commit to nbraud/salt that referenced this issue Jul 17, 2018

module/acme: Do not exit early when the certificate already exists
Otherwise, acme.cert might fail to cert permissions on the certificate.

Close saltstack#48626

nbraud added a commit to nbraud/salt that referenced this issue Jul 17, 2018

module/acme: Do not exit early when the certificate already exists
Otherwise, acme.cert might fail to cert permissions on the certificate.

Close saltstack#48626
@Ch3LL

This comment has been minimized.

Contributor

Ch3LL commented Jul 18, 2018

Thanks for the PR! As it looks like your PR does indeed fix the issue I will close this issue and keep the conversation in the PR but let me know if this needs to be re-opened.

@Ch3LL Ch3LL closed this Jul 18, 2018

@Ch3LL Ch3LL added this to the Approved milestone Jul 18, 2018

cro added a commit to cro/salt that referenced this issue Jul 26, 2018

module/acme: Do not exit early when the certificate already exists
Otherwise, acme.cert might fail to cert permissions on the certificate.

Close saltstack#48626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment