Skip to content
This repository has been archived by the owner on Jun 25, 2024. It is now read-only.

Validating dataplane/controlplane TLS consistency #821

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions api/v1beta1/openstackdataplanenodeset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package v1beta1

import (
"context"
"fmt"

"golang.org/x/exp/slices"
"sigs.k8s.io/controller-runtime/pkg/client"

infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
Expand Down Expand Up @@ -290,3 +292,49 @@ func (r *OpenStackDataPlaneNodeSetSpec) duplicateNodeCheck(nodeSetList *OpenStac

return
}

// Compare TLS settings of control plane and data plane
// if control plane name is specified attempt to retrieve it
// otherwise get any control plane in the namespace
func (r *OpenStackDataPlaneNodeSetSpec) ValidateTLS(namespace string, reconcilerClient client.Client, ctx context.Context) error {
var err error
controlPlanes := openstackv1.OpenStackControlPlaneList{}
opts := client.ListOptions{
Namespace: namespace,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

In the case where the control plane is not found, we should requeue and wait, right?

Copy link
Contributor Author

@jpodivin jpodivin Apr 18, 2024

Choose a reason for hiding this comment

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

Based on your comments in JIRA I though we want to ignore this case and continue uninterrupted?

If only one exists, use that one. If more than one exists, fail with message. If not control plane exists, do nothing.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah good point. I forgot about the no control plane case. @slagle - this is a legit case, right?

// Attempt to get list of all ControlPlanes fail if that isn't possible
if err = reconcilerClient.List(ctx, &controlPlanes, &opts); err != nil {
return err
}
// Verify TLS status of control plane only if there is a single one
// report error if there are multiple, or proceed if there are none
if len(controlPlanes.Items) > 1 {
err = fmt.Errorf("multiple control planes found in the namespace %s", namespace)
} else if len(controlPlanes.Items) == 1 {
controlPlane := controlPlanes.Items[0]
fieldErr := r.TLSMatch(controlPlane)
if fieldErr != nil {
err = fmt.Errorf("%s", fieldErr.Error())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, if there is no control plane , we should requeue and wait, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto

}

return err
}

// Do TLS flags match in control plane ingress, pods and data plane
func (r *OpenStackDataPlaneNodeSetSpec) TLSMatch(controlPlane openstackv1.OpenStackControlPlane) *field.Error {

if controlPlane.Spec.TLS.Ingress.Enabled != r.TLSEnabled || controlPlane.Spec.TLS.PodLevel.Enabled != r.TLSEnabled {

return field.Forbidden(
field.NewPath("spec.tlsEnabled"),
fmt.Sprintf(
"TLS settings on Data Plane node set and Control Plane %s do not match, Node set: %t Control Plane Ingress: %t Control Plane PodLevel: %t",
controlPlane.Name,
r.TLSEnabled,
controlPlane.Spec.TLS.Ingress.Enabled,
controlPlane.Spec.TLS.PodLevel.Enabled))
}
return nil
}
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,14 @@ rules:
- patch
- update
- watch
- apiGroups:
- core.openstack.org
resources:
- openstackcontrolplanes
verbs:
- get
- list
- watch
- apiGroups:
- core.openstack.org
resources:
Expand Down
11 changes: 11 additions & 0 deletions controllers/openstackdataplanedeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) GetLogger(ctx context.Context)
//+kubebuilder:rbac:groups=discovery.k8s.io,resources=endpointslices,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=cert-manager.io,resources=issuers,verbs=get;list;watch;
//+kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=core.openstack.org,resources=openstackcontrolplanes,verbs=get;list;watch;

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down Expand Up @@ -183,6 +184,16 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}
if err = nodeSetInstance.Spec.ValidateTLS(instance.GetNamespace(), r.Client, ctx); err != nil {
Log.Info("error while comparing TLS settings of nodeset %s with control plane: %w", nodeSet, err)
instance.Status.Conditions.MarkFalse(
dataplanev1.SetupReadyCondition,
condition.ErrorReason,
condition.SeverityError,
dataplanev1.DataPlaneNodeSetErrorMessage,
err.Error())
return ctrl.Result{}, err
}
nodeSets.Items = append(nodeSets.Items, *nodeSetInstance)
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
k8s.io/api v0.28.10
k8s.io/apimachinery v0.28.10
k8s.io/client-go v0.28.10
k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0
sigs.k8s.io/controller-runtime v0.16.6
)

Expand Down Expand Up @@ -113,7 +114,6 @@ require (
k8s.io/component-base v0.28.10 // indirect
k8s.io/klog/v2 v2.120.1 // indirect
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 // indirect
sigs.k8s.io/gateway-api v0.8.0 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
Expand Down
33 changes: 33 additions & 0 deletions tests/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

dataplanev1 "github.com/openstack-k8s-operators/dataplane-operator/api/v1beta1"
infrav1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
Expand Down Expand Up @@ -344,6 +345,38 @@ func DefaultDataplaneGlobalService(name types.NamespacedName) map[string]interfa
}
}

// Create simple OpenStackControlPlane
func CreateOpenStackControlPlane(name types.NamespacedName, tlsEnabled bool) client.Object {

raw := map[string]interface{}{
"apiVersion": "core.openstack.org/v1beta1",
"kind": "OpenStackControlPlane",
"metadata": map[string]interface{}{
"name": name.Name,
"namespace": name.Namespace,
},
"spec": map[string]interface{}{
"secret": "osp-secret",
"storageClass": "local-storage",
"tls": map[string]interface{}{
"ingress": map[string]interface{}{
"enabled": tlsEnabled,
"ca": map[string]interface{}{
"duration": "100h",
},
"cert": map[string]interface{}{
"duration": "10h",
},
},
"podLevel": map[string]interface{}{
"enabled": tlsEnabled,
},
},
},
}
return th.CreateUnstructured(raw)
}

// Get resources

// Retrieve OpenStackDataPlaneDeployment and check for errors
Expand Down
Loading