Skip to content

Barbican Support for Luna HSM#168

Merged
openshift-merge-bot[bot] merged 1 commit into
openstack-k8s-operators:mainfrom
mauricioharley:add_thales_luna_support
Dec 6, 2024
Merged

Barbican Support for Luna HSM#168
openshift-merge-bot[bot] merged 1 commit into
openstack-k8s-operators:mainfrom
mauricioharley:add_thales_luna_support

Conversation

@mauricioharley

@mauricioharley mauricioharley commented Oct 9, 2024

Copy link
Copy Markdown
Contributor

This patch is a Work in Progress to support Thales Luna HSMs on Barbican v18.

It templates the Luna HSM configuration into Barbican variables.

Comment thread api/v1beta1/common_types.go Outdated
Comment thread api/v1beta1/common_types.go Outdated
Comment thread api/v1beta1/common_types.go Outdated

@vakwetu vakwetu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Next step of course is to take what you have here and add to the relevant specs.

Then you need to find where the config files are generated/templated and make the changes. See https://github.com/openstack-k8s-operators/keystone-operator/pull/479/files# as an example of what Dave is doing on the federation side.

Comment thread api/v1beta1/common_types.go
Comment thread api/v1beta1/barbicanapi_types.go Outdated
Comment thread api/v1beta1/barbicanapi_types.go Outdated
Comment thread api/v1beta1/barbicanworker_types.go Outdated
Comment thread controllers/barbicanapi_controller.go Outdated
Comment thread controllers/barbicanapi_controller.go Outdated
Comment thread controllers/barbicanapi_controller.go Outdated
Comment thread controllers/barbicanapi_controller.go Outdated
Comment thread controllers/barbicanworker_controller.go Outdated
@mauricioharley mauricioharley changed the title Initial support for Thales Luna HSM Barbican Support for Luna HSM Oct 11, 2024
Comment thread templates/barbican/config/00-default.conf Outdated
Comment thread api/v1beta1/barbicanapi_types.go Outdated
Comment thread templates/barbican/config/barbican-api-config.json
Comment thread api/v1beta1/common_types.go Outdated
// +kubebuilder:validation:Optional
// The HSM certificates. The map's key is the OpenShift secret storing the certificate, and
// the value is the mounting point (e.g., "luna-certificates": "/usr/local/luna/config/certs").
HSMCertificates map[string]string `json:"hsmCertificates"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can there be multiple hsm certificates involved? because if I read the PR correct, only one would be mounted . If if only one cert secret is used, can't we just mount them to a fixed patch, instead of making the path configurable?
If we need it to be configurable, why not have a list of a sub struct type with secretName and mountPath ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also what is the expected content of the secret? specific keys for referencing cert/key?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This has been modified. We can have multiple certs but they will all be contained in a single secret - which will be mounted to a specific directory (mountpoint). The contents of the secret will be all the files that will be created in that directory.

Comment thread controllers/barbicanworker_controller.go Outdated
Comment thread controllers/barbicanworker_controller.go Outdated
Comment thread templates/barbican/config/00-default.conf Outdated
Comment thread templates/barbican/config/Chrystoki.conf Outdated
Comment thread templates/barbican/config/barbican-api-config.json Outdated
Comment thread templates/barbican/config/barbican-worker-config.json Outdated
Comment thread api/v1beta1/barbican_types.go Outdated
Comment thread api/v1beta1/common_types.go Outdated
Comment thread api/v1beta1/common_types.go
Comment thread pkg/barbican/volumes.go
@mauricioharley mauricioharley force-pushed the add_thales_luna_support branch 3 times, most recently from ef4921e to 434b386 Compare November 15, 2024 16:06
@vakwetu

vakwetu commented Nov 15, 2024

Copy link
Copy Markdown
Contributor

Looks like kuttl tests are passing. but the deploy-test there is failing while waiting on galera.

@vakwetu

vakwetu commented Nov 15, 2024

Copy link
Copy Markdown
Contributor

/test barbican-operator-build-deploy-kuttl

1 similar comment
@vakwetu

vakwetu commented Nov 18, 2024

Copy link
Copy Markdown
Contributor

/test barbican-operator-build-deploy-kuttl

@mauricioharley

Copy link
Copy Markdown
Contributor Author

/test barbican-operator-build-deploy-kuttl

@mauricioharley

Copy link
Copy Markdown
Contributor Author

/retest

@mauricioharley mauricioharley force-pushed the add_thales_luna_support branch 3 times, most recently from 2723ff1 to 693987b Compare December 2, 2024 19:39
@vakwetu

vakwetu commented Dec 2, 2024

Copy link
Copy Markdown
Contributor

/test barbican-operator-build-deploy-kuttl

Comment thread controllers/barbican_controller.go
hold-the-node
Signed-off-by: Mauricio Harley <mharley@redhat.com>
Co-authored-by: Ade Lee <alee@redhat.com>
Co-authored-by: Grzegorz Grasza <xek@redhat.com>
@xek xek force-pushed the add_thales_luna_support branch from 693987b to d84132c Compare December 4, 2024 11:42
mkdir -p "$(dirname "$TMP_DIR/$crd")"
git show "$BASE_REF:$crd" > "$TMP_DIR/$crd"
$CHECKER check-manifests \
--disabled-validators=NoBools,NoNewRequiredFields \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK - so this was just for testing. We want to remove this and then explicitly waive the test failures.

@xek xek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Dec 6, 2024
@openshift-ci

openshift-ci Bot commented Dec 6, 2024

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mauricioharley, xek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot Bot merged commit cc2b30a into openstack-k8s-operators:main Dec 6, 2024
@vakwetu

vakwetu commented Dec 6, 2024

Copy link
Copy Markdown
Contributor

Just to add a comment here about the CRD checker.

This patch was accidentally merged without removing the changes to the crd-schema-checker. We will re-enable them in a follow-on commit.

The checker flagged a bunch of bools that were added in the pkcs11 spec. These bools are in fact really bools that would be set as such in the barbican.conf file. So the errors here would have been waived.

Further, the checker flagged a bunch of new required fields in the PKCS11 struct. This is OK because the PKCS11 struct itself is new and it is included into our existing structs as an optional pointer to a PKCS11 struct. Existing CRs will not have this field defined, and upon update, will instantiate that pointer as a nil value. Which won't trigger any issues with required fields in the struct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants