-
Notifications
You must be signed in to change notification settings - Fork 37
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
[tlse] TLS database connection #349
[tlse] TLS database connection #349
Conversation
/hold |
/test cinder-operator-build-deploy-tempest |
controllers/cinder_controller.go
Outdated
customData := map[string]string{cinder.CustomConfigFileName: instance.Spec.CustomServiceConfig} | ||
customData := map[string]string{ | ||
cinder.CustomConfigFileName: instance.Spec.CustomServiceConfig, | ||
"my.cnf": db.GetDatabaseClientConfig(tlsCfg), //(mschuppert) for now just get the default my.cnf |
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.
I think I understand the code, with hints from reading the corresponding PR in glance-operator, but I don't understand the "for now just get the default my.cnf" comment. For now? Is this a TODO that needs to be revisited?
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.
right now we request the default my.cnf by either tlsCfg being nil
, or &tls.Service{}
. GetDatabaseClientConfig() also supports to add client cert config, but there is additional work needed to support client certs. thats what I meant with the default config.
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.
Ack, makes sense now!
controllers/cinder_controller.go
Outdated
customData := map[string]string{cinder.CustomConfigFileName: instance.Spec.CustomServiceConfig} | ||
customData := map[string]string{ | ||
cinder.CustomConfigFileName: instance.Spec.CustomServiceConfig, | ||
"my.cnf": db.GetDatabaseClientConfig(tlsCfg), //(mschuppert) for now just get the default my.cnf |
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.
Also, I prefer to avoid hard-coding the same string in multiple locations. Consider defining (and using) this in pkg/cinder/const.go along with the other XxxFileName constants:
MyCnfFileName = "my.cnf"
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.
ok, I can do that
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.
done
The my.cnf file gets added to the secret holding the service configs. The content of my.cnf is centrally managed in the mariadb-operator and retrieved calling db.GetDatabaseClientConfig(tlsCfg) Depends-On: openstack-k8s-operators/mariadb-operator#190 Depends-On: openstack-k8s-operators/mariadb-operator#191 Jira: OSPRH-4547
@stuggi: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ASBishop, stuggi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c46d63a
into
openstack-k8s-operators:main
…rators/fix_up Donot trigger dependent jobs on irrelevant files changes
The my.cnf file gets added to the secret holding the service configs. The content of my.cnf is centrally managed in the mariadb-operator and retrieved calling db.GetDatabaseClientConfig(tlsCfg)
Depends-On: openstack-k8s-operators/mariadb-operator#190
Depends-On: openstack-k8s-operators/mariadb-operator#191
Jira: OSPRH-4547