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

Support of additional SAN for etcd #49

Merged
merged 29 commits into from
Jan 11, 2023
Merged

Support of additional SAN for etcd #49

merged 29 commits into from
Jan 11, 2023

Conversation

danopt
Copy link
Member

@danopt danopt commented Nov 4, 2022

Fixes #48

To support etcd the IP addresses and the loopback address has to be defined in the SAN.

To achieve this a temporary variable ca_san will be defined with set_fact. I also implemented the support of custom additional SANs with the variable ca_san_custom.

This is the value of the variable ca_san, after it is defined with set_fact and the whitespaces are removed with regex_replace():
`DNS:rockylinux-8-03,DNS:rockylinux-8-03,DNS:vm-3,IP:127.0.0.1,IP:10.0.53.15,IP:10.0.53.15,IP:10.0.53.15'

The string of ca_san_custom variable has to begin with a ',' and could look like this to add the IP address of a second NIC (will be combined with ca_san):

    ca_san_custom:  >-
      ,
      {%- for host in vars['play_hosts'] -%}
        IP:{{ hostvars[inventory_hostname].ansible_eth1.ipv4.address }}
        {%- if not loop.last -%}
          ,
        {%- endif -%}
      {%- endfor -%}

@danopt danopt requested a review from widhalmt November 4, 2022 13:12
@danopt danopt changed the title Support of SAN for etcd Support of additional SAN for etcd Nov 4, 2022
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

I got a few annotations. To be honest, I couldn't test the whle code so there might still be typos in my version. But I hope I could show what I intended.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
danopt and others added 5 commits November 9, 2022 10:05
Co-authored-by: Thomas Widhalm <[email protected]>
Co-authored-by: Thomas Widhalm <[email protected]>
Co-authored-by: Thomas Widhalm <[email protected]>
Co-authored-by: Thomas Widhalm <[email protected]>
@widhalmt widhalmt added this to the 0.1.0 milestone Dec 14, 2022
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

Could you have a look at my comment? Maybe I misunderstood your change but if I see it correctly it might break any option of using the role for anything else than etcd when the etcd option is set. That's ok but please make it clear in the Readme.

tasks/main.yml Outdated Show resolved Hide resolved
* Check for instance key
* Use new ca_client_ca_dir
* Add check for server certificate

fixes #51
@danopt danopt marked this pull request as draft December 15, 2022 08:55
@danopt danopt marked this pull request as ready for review December 20, 2022 12:08
@danopt danopt requested a review from widhalmt December 21, 2022 18:11
@danopt danopt marked this pull request as draft December 27, 2022 14:16
@danopt
Copy link
Member Author

danopt commented Dec 27, 2022

Changes:

  • Added molecule verification tasks

@danopt danopt marked this pull request as ready for review December 27, 2022 15:21
@danopt danopt marked this pull request as draft December 27, 2022 16:37
@danopt danopt marked this pull request as ready for review December 30, 2022 16:45
@danopt
Copy link
Member Author

danopt commented Jan 9, 2023

Fixes #56

@danopt danopt self-assigned this Jan 9, 2023
@danopt danopt added the feature New feature or request label Jan 9, 2023
@danopt danopt marked this pull request as draft January 9, 2023 12:41
@danopt danopt marked this pull request as ready for review January 11, 2023 09:54
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

This looks very, very good! Thanks for the great contribution!

@danopt
Copy link
Member Author

danopt commented Jan 11, 2023

Thx. Not a problem at all. ;-)

@danopt danopt merged commit 858b8c4 into main Jan 11, 2023
@danopt danopt deleted the feature-san branch January 11, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support of additional SAN for etcd
2 participants