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: postgresql_cert_name didn't work properly, using this parameter #102

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

fila43
Copy link
Collaborator

@fila43 fila43 commented Oct 14, 2024

postgresql_cert_name parameter caused an error. Because there is a missing variable __pg_server_crt definition in the correct context. This commit also enhances the description of postgresql_cern_name variable.

Issue Tracker Tickets (Jira or BZ if any): SYSROLES-157

caused an error. Because there was a missing variable __pg_server_crt
definition in the correct context. This commit also enhances the description
of postgresql_cern_name variable.

Related: SYSROLES-157
@fila43
Copy link
Collaborator Author

fila43 commented Oct 14, 2024

[citest]

@richm
Copy link
Contributor

richm commented Oct 14, 2024

@fila43 we'll need to add a new test for this, or modify one of the existing tests in tests/ e.g. https://github.com/linux-system-roles/postgresql/blob/main/tests/tests_certificate.yml

@fila43
Copy link
Collaborator Author

fila43 commented Oct 14, 2024

@richm I would prefer to add the new one.

@fila43
Copy link
Collaborator Author

fila43 commented Oct 14, 2024

[citest]

@@ -0,0 +1,50 @@
---
- name: Generate certificate using certificate role
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't have to be a separate play - this can be a task in the Test PostgreSQL server with ssl support using certificate role play

There is a way to use the certificate role in test mode which makes it easier to use for system roles testing.

        - name: Generate certificates
          include_role:
            name: fedora.linux_system_roles.certificate
          vars:
            certificate_requests:
              - name: /tmp/mycert
                dns: www.example.com
                ca: self-sign
            certificate_test_mode: true

then you don't need the task for Stop tracking certificate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I didn't know it.

@fila43
Copy link
Collaborator Author

fila43 commented Oct 14, 2024

[citest]

that: >-
"SSL connection" in result.stdout
always:
- name: Stop tracking certificate
Copy link
Contributor

Choose a reason for hiding this comment

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

When using the certificate role with certificate_test_mode: true you don't need to stop tracking - the certificate role does this for you https://github.com/linux-system-roles/certificate/blob/main/tasks/main.yml#L184

@richm
Copy link
Contributor

richm commented Oct 15, 2024

@fila43 all Ansible YAML files must have a .yml extension, not a .yaml extension

@spetrosi
Copy link
Contributor

[citest]

@richm
Copy link
Contributor

richm commented Oct 15, 2024

The certificate role test mode doesn't work with name: using a full path. I'm trying to figure out an alternate way to do this.

@fila43
Copy link
Collaborator Author

fila43 commented Oct 15, 2024

[citest]

@fila43
Copy link
Collaborator Author

fila43 commented Oct 15, 2024

The certificate role test mode doesn't work with name: using a full path. I'm trying to figure out an alternate way to do this.

My current version works locally, but since I am not an ansible expert, I don't know if it satisfies all the rules.

@richm
Copy link
Contributor

richm commented Oct 15, 2024

The certificate role test mode doesn't work with name: using a full path. I'm trying to figure out an alternate way to do this.

My current version works locally, but since I am not an ansible expert, I don't know if it satisfies all the rules.

Ah - because you aren't using /tmp - name: /etc/pki/tls/certs/mycert
using /tmp fails with an selinux AVC

name: fedora.linux_system_roles.certificate
vars:
certificate_requests:
- name: /etc/pki/tls/certs/mycert
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: /etc/pki/tls/certs/mycert
- name: /etc/pki/tls/certs/postgresql_test

So that if something goes wrong with the test, and doesn't clean up, we know which test this is

vars:
__test_clean_instance: false
__test_check_unix_socket: false
postgresql_cert_name: /etc/pki/tls/certs/mycert
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
postgresql_cert_name: /etc/pki/tls/certs/mycert
postgresql_cert_name: /etc/pki/tls/certs/postgresql_test

@fila43
Copy link
Collaborator Author

fila43 commented Oct 15, 2024

The certificate role test mode doesn't work with name: using a full path. I'm trying to figure out an alternate way to do this.

My current version works locally, but since I am not an ansible expert, I don't know if it satisfies all the rules.

Ah - because you aren't using /tmp - name: /etc/pki/tls/certs/mycert using /tmp fails with an selinux AVC

Yes, but there is a missing check mechanism in the certificate role. Usually, it gets stuck in the Ensure certificate requests task without any message.

@fila43
Copy link
Collaborator Author

fila43 commented Oct 15, 2024

[citest]

@richm richm merged commit df7a745 into linux-system-roles:main Oct 16, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants