-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add support for TLS configuration at MAAS charm #220
Conversation
…ig-tls enable if tls_mode=passthrough
…iles. Ensure the files exist in the proper place before configuring TLS
…, as the types of these are checked on deployment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I want to update tls_mode to disabled again? We need to add that logic to our charm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thoughts: We need to run the configure TLS on config changed so I am going to remove the -1 until we got this fixed. The way it is right now it's not going to update the TLS when user decides to do it. We should also think if adding it only on config_changed is enough. On setup phase the config_changed event is triggered but MAAS is not initialized yet. So my feeling is that we need to add it there with a check on maas initialization plus to a place that follows the first maas_initialization. I hope it makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1
…ng TLS. Check whether TLS is enabled before enabling/disabling it
…ed, use this to check for initialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small improvements. Other than that, I think we are good to go 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1 🚀
(cherry picked from commit ddbc0fd)
Adds three config items:
ssl_cert_content
,ssl_key_content
andssl_cacert_content
, as well as a helper function to runmaas config-tls enable...