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

Add TLS Support to barbican operator #55

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

vakwetu
Copy link
Contributor

@vakwetu vakwetu commented Dec 13, 2023

This adds the changes needed to enable TLS-E in barbican.

Copy link

openshift-ci bot commented Jan 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vakwetu

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

d34dh0r53 added a commit to d34dh0r53/openstack-operator that referenced this pull request Jan 16, 2024
Creates certs for k8s service of the service operator when
spec.tls.endpoint.internal.enabled: true

For a service like nova which talks to multiple service internal
endpoints, this has to be set for each of them for, like:

~~~
  customServiceConfig: |
    [keystone_authtoken]
    insecure = true
    [placement]
    insecure = true
    [neutron]
    insecure = true
    [glance]
    insecure = true
    [cinder]
    insecure = true
~~~

Depends-On: openstack-k8s-operators/lib-common#428
Depends-On: openstack-k8s-operators#620
Depends-On: openstack-k8s-operators/barbican-operator#55

Jira: OSPRH-2349
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

in general looks good. might want to add envTest and kuttl tests, like we added to the other operators. Can you also add a sample for tls

}
}

// TODO(alee) should this validation occur in an if statement? what happens when tls is not enabled?
Copy link
Contributor

Choose a reason for hiding this comment

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

certsHash would be calculated for the empty certHashes https://github.com/openstack-k8s-operators/lib-common/blob/main/modules/common/tls/tls.go#L166 as both endpoints are not enabled for tls

instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)

// TODO(alee) Figure out how serviceLabels are used and what must be in them
Copy link
Contributor

Choose a reason for hiding this comment

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

serviceLabels is basically the base labels to be set on all resources created by the controller to easy query/identify them. depending on which resource is created they get extended with additional, e.g. k8s svc will have the serviceLabels + the endpoint identifier.

@@ -634,9 +696,38 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
return ctrlResult, nil
}

Log.Info(fmt.Sprintf("[API] Getting input hash '%s'", instance.Name))
Copy link
Contributor

Choose a reason for hiding this comment

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

should we keep this log?

controllers/barbicanapi_controller.go Outdated Show resolved Hide resolved
@d34dh0r53
Copy link
Contributor

d34dh0r53 commented Feb 5, 2024

/wip

These tests will determine the functionality of TLSe within the
barbican-operator.
@d34dh0r53
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 9, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 3b43913 into openstack-k8s-operators:main Feb 9, 2024
6 checks passed
d34dh0r53 added a commit to d34dh0r53/openstack-operator that referenced this pull request Feb 12, 2024
spec.tls.endpoint.internal.enabled: true

For a service like nova which talks to multiple service internal
endpoints, this has to be set for each of them for, like:

~~~
  customServiceConfig: |
    [keystone_authtoken]
    insecure = true
    [placement]
    insecure = true
    [neutron]
    insecure = true
    [glance]
    insecure = true
    [cinder]
    insecure = true
~~~

Depends-On: openstack-k8s-operators/lib-common#428
Depends-On: openstack-k8s-operators#620
Depends-On: openstack-k8s-operators/barbican-operator#55

Jira: OSPRH-2349
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.

4 participants