Conversation
|
|
|
|
|
|
875b55d to
8285fe7
Compare
|
|
|
Kidswiss
left a comment
There was a problem hiding this comment.
The code looks good. Just a few remarks.
I haven't tested it yet, but will do some tests later.
Search for newly created namespace with openbao name included. Use bao operator init -tls-skip-verify to initialize the cluster manually with kubectl exec command.
One idea: this should probably be a post-install hook in the helm chart, since it's still part of the provisioning.
| // VSHNOpenBaoServiceSpec contains OpenBao DBaaS specific properties | ||
| type VSHNOpenBaoServiceSpec struct { | ||
| // +kubebuilder:validation:Enum=<TBD> | ||
| // +kubebuilder:default=<TBD> |
There was a problem hiding this comment.
Should probably be defined before release.
There was a problem hiding this comment.
The Version field was just a placeholder. The version is set in Components Appcat.
| Version string `json:"version,omitempty"` | ||
|
|
||
| // Openbaosettings contains additional OpenBao settings. | ||
| Openbaosettings string `json:"openbaosettings,omitempty"` |
There was a problem hiding this comment.
Is this really just a string? Or only a placeholder? Usually settings are modelled with a map[string]any
There was a problem hiding this comment.
That was a placeholder too, so I removed it for now. We'll add it back later probably with better defined type.
| withIssuerOfTypeSelfSigned(), | ||
| }, | ||
| } | ||
| selfSignedIssuer := createIssuer(ns, serviceName+tlsSelfSignedIssuerSuffix, selfSignedIssuerOpts) |
There was a problem hiding this comment.
I guess common.CreateCertificate() wasn't flexible enough to handle this?
I'm wondering if it would make sense to refactor it make it more flexible.
There was a problem hiding this comment.
I didn't know common.CreateCertificate() exists. I think we were looking at Redis and certificates are created differently there.
I update the code, now it's using common.CreateCertificate()
There was a problem hiding this comment.
Yeah, some older services might not use the common library because they were migrated from legacy crossplane compositions and are thus stuck in a kind of limbo.
|
|
Kidswiss
left a comment
There was a problem hiding this comment.
I tried to create an instance, but I struggle to get the cluster initialized properly.
My steps:
k -n vshn-openbao-openbao-test-v54vf exec -it openbao-test-v54vf-0 -- sh
bao operator init -tls-skip-verify
bao operator unseal -tls-skip-verify
k -n vshn-openbao-openbao-test-v54vf exec -it openbao-test-v54vf-1 -- sh
bao operator unseal -tls-skip-verify
Unseal Key (will be hidden):
Error unsealing: Error making API request.
URL: PUT https://127.0.0.1:8200/v1/sys/unseal
Code: 400. Errors:
* Vault is not initialized
The first pod looks fine, the others throw this error. According to the openbao docs bao operator init has to be run only once.
Checking the config within the pod I see this:
cat userconfig/openbao-hcl-config/config.hcl
ui = true
log_level = "info"
log_format = "json"
cluster_name = "openbao-test-v54vf"
api_addr = "https://openbao-test-v54vf:8200"
cluster_addr = "https://openbao-test-v54vf:8201"
listener "tcp" {
address = "[::]:8200"
cluster_address = "[::]:8201"
tls_disable = false
tls_cert_file = "/openbao/userconfig/openbao-tls/tls.crt"
tls_key_file = "/openbao/userconfig/openbao-tls/tls.key"
}
storage "raft" {
path = "/openbao/data"
}
Isn't there supposed to be something to tell the instance where its peers are?
| type VSHNOpenBaoSizeSpec struct { | ||
|
|
There was a problem hiding this comment.
This one seems unused. At least according to the linter.
| // GetPDBLabels returns the labels to be used for the PodDisruptionBudget | ||
| // it should match one unique label od pod running in instanceNamespace | ||
| // without this, the PDB will match all pods | ||
| func (v *VSHNOpenBao) GetPDBLabels() map[string]string { |
There was a problem hiding this comment.
Does the helm chart you're using already deploy PDBs? If not setting the labels to match the pods and then calling common.AddPDBSettings() will automagically deploy them. But I think the instances parameter needs to be set for them to work...
| "server": map[string]any{ | ||
| "ha": map[string]any{ | ||
| "enabled": true, | ||
| "config": "# Config provided via external file\n", |
There was a problem hiding this comment.
I'm wondering why the config is provided via an external file?
Summary
Initialize OpenBao Service. The goal is to provide users with an option to setup HA OpenBao Cluster using Appcat.
Initial implementation contains:
The service is not yet ready for production use. Further development will follow.
How to test?
vshnopenbaos.vshn.appcat.vshn.ioresource.openbaoname included. Usebao operator init -tls-skip-verifyto initialize the cluster manually withkubectl execcommand.Checklist
/mergecomment.Component PR: vshn/component-appcat#1128