Skip to content

Commit

Permalink
Add default values for the 'cloudName' and 'identityRef' fields
Browse files Browse the repository at this point in the history
Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud>
  • Loading branch information
michal-gubricky committed Feb 19, 2024
1 parent 9a65361 commit 284ff65
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 42 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ clusterctl init --infrastructure openstack
To enable communication between the CSPO and the Cluster API Provider for OpenStack (CAPO) with the OpenStack API, it is necessary to generate a secret containing the access data (clouds.yaml).
Ensure that this secret is located in the identical namespace as the other Custom Resources.

> [!NOTE]
> The default value of `cloudName` is configured as `openstack`. This setting can be overridden by including the `cloudName` key in the secret. Also, be aware that the name of the secret is expected to be `openstack` unless it is not set differently in OpenStackClusterStackReleaseTemplate in `identityRef.name` field.
```bash
kubectl create secret generic <my-cloud-secret> --from-file=clouds.yaml=path/to/clouds.yaml

Expand Down
2 changes: 1 addition & 1 deletion Tiltfile
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def deploy_cspo():

def create_secret():
cmd = "cat .secret.yaml | {} | kubectl apply -f -".format(envsubst_cmd)
local_resource('supersecret', cmd, labels=["clouds-yaml-secret"])
local_resource('supersecret', cmd, labels=["clouds-yaml-secret"])

def cspo_template():
cmd = "cat .cspotemplate.yaml | {}".format(envsubst_cmd)
Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/conditions_const.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ const (
)

const (
// CloudNameAvailableCondition is used when cloud name is available.
CloudNameAvailableCondition = "CloudNameAvailable"

// CloudAvailableCondition is used when cloud is available.
CloudAvailableCondition = "CloudAvailable"

Expand Down
5 changes: 2 additions & 3 deletions api/v1alpha1/openstackclusterstackrelease_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ import (

// OpenStackClusterStackReleaseSpec defines the desired state of OpenStackClusterStackRelease.
type OpenStackClusterStackReleaseSpec struct {
// CloudName is the name of the cloud to use from the cloud's secret.
// +kubebuilder:validation:MinLength=1
CloudName string `json:"cloudName"`
// IdentityRef is a reference to a identity to be used when reconciling this cluster
// +optional
// +kubebuilder:default:={kind: "Secret", name: "openstack"}
IdentityRef *apiv1alpha7.OpenStackIdentityReference `json:"identityRef"`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,10 @@ spec:
description: OpenStackClusterStackReleaseSpec defines the desired state
of OpenStackClusterStackRelease.
properties:
cloudName:
description: CloudName is the name of the cloud to use from the cloud's
secret.
minLength: 1
type: string
identityRef:
default:
kind: Secret
name: openstack
description: IdentityRef is a reference to a identity to be used when
reconciling this cluster
properties:
Expand All @@ -75,9 +73,6 @@ spec:
- kind
- name
type: object
required:
- cloudName
- identityRef
type: object
status:
description: OpenStackClusterStackReleaseStatus defines the observed state
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ spec:
description: OpenStackClusterStackReleaseSpec defines the desired
state of OpenStackClusterStackRelease.
properties:
cloudName:
description: CloudName is the name of the cloud to use from
the cloud's secret.
minLength: 1
type: string
identityRef:
default:
kind: Secret
name: openstack
description: IdentityRef is a reference to a identity to be
used when reconciling this cluster
properties:
Expand All @@ -72,9 +70,6 @@ spec:
- kind
- name
type: object
required:
- cloudName
- identityRef
type: object
required:
- spec
Expand Down
11 changes: 6 additions & 5 deletions config/cspo/cspotemplate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ metadata:
namespace: cluster
spec:
template:
spec:
cloudName: "${CLOUD_NAME}"
identityRef:
kind: Secret
name: "${SECRET_NAME}"
spec: {}
# Field identityRef is optional and its default values ​​are as follows:
# identityRef.kind: "Secret", identityRef.name: "openstack"
# identityRef:

Check warning on line 11 in config/cspo/cspotemplate.yaml

View workflow job for this annotation

GitHub Actions / Lint Pull Request

11:7 [comments-indentation] comment not indented like content
# kind: Secret
# name: "<your-secret-name>"
7 changes: 6 additions & 1 deletion config/cspo/secret.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
apiVersion: v1
data:
# The default value of `cloudName` is configured as `openstack`.
# This can be overridden by including the `cloudName` key in this secret.
# cloudName: "openstack"
clouds.yaml: ${ENCODED_CLOUDS_YAML}
kind: Secret
metadata:
labels:
clusterctl.cluster.x-k8s.io/move: "true"
name: "${SECRET_NAME}"
# Note: Value of the field `name` must be the same as the value of field
# `identityRef.name` in OpenStackClusterStackReleaseTemplate object.
name: "openstack"
namespace: cluster
3 changes: 2 additions & 1 deletion examples/cspotemplate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ metadata:
spec:
template:
spec:
cloudName: <name of the cloud to use from the clouds secret>
# Field identityRef is optional and its default values ​​are as follows:

Check warning on line 8 in examples/cspotemplate.yaml

View workflow job for this annotation

GitHub Actions / Lint Pull Request

8:5 [comments-indentation] comment not indented like content
# identityRef.kind: "Secret", identityRef.name: "openstack"
identityRef:
kind: Secret
name: <my-secret>
48 changes: 45 additions & 3 deletions internal/controller/openstackclusterstackrelease_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/release"
apiv1alpha1 "github.com/sovereignCloudStack/cluster-stack-provider-openstack/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -60,6 +61,7 @@ type NodeImages struct {
}

const (
cloudNameSecretKey = "cloudName"
metadataFileName = "metadata.yaml"
nodeImagesFileName = "node-images.yaml"
waitForOpenStackNodeImageReleasesBecomeReady = 30 * time.Second
Expand Down Expand Up @@ -174,8 +176,22 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context,
}

osnirName := fmt.Sprintf("%s-%s-%s", nameWithoutVersion, openStackNodeImage.CreateOpts.Name, nodeImageVersion)
cloudName, err := r.getCloudNameFromSecret(ctx, openstackclusterstackrelease.Namespace, openstackclusterstackrelease.Spec.IdentityRef.Name)
if err != nil {
if apierrors.IsNotFound(err) {
conditions.MarkFalse(openstackclusterstackrelease,
apiv1alpha1.CloudNameAvailableCondition,
apiv1alpha1.SecretNotFoundReason,
clusterv1beta1.ConditionSeverityError,
err.Error(),
)
record.Warnf(openstackclusterstackrelease, "SecretNotFound", err.Error())
logger.Error(err, "failed to get secret")
return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
}
}

if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, openStackNodeImage, ownerRef); err != nil {
if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, cloudName, openStackNodeImage, ownerRef); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to create or update OpenStackNodeImageRelease %s/%s: %w", openstackclusterstackrelease.Namespace, osnirName, err)
}
}
Expand Down Expand Up @@ -218,7 +234,7 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context,
return ctrl.Result{}, nil
}

func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error {
func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName, cloudName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error {
openStackNodeImageRelease := &apiv1alpha1.OpenStackNodeImageRelease{}

err := r.Get(ctx, types.NamespacedName{Name: osnirName, Namespace: openstackclusterstackrelease.Namespace}, openStackNodeImageRelease)
Expand Down Expand Up @@ -250,7 +266,7 @@ func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImag
}
openStackNodeImageRelease.SetOwnerReferences([]metav1.OwnerReference{*ownerRef})
openStackNodeImageRelease.Spec.Image = openStackNodeImage
openStackNodeImageRelease.Spec.CloudName = openstackclusterstackrelease.Spec.CloudName
openStackNodeImageRelease.Spec.CloudName = cloudName
openStackNodeImageRelease.Spec.IdentityRef = openstackclusterstackrelease.Spec.IdentityRef

if err := r.Create(ctx, openStackNodeImageRelease); err != nil {
Expand Down Expand Up @@ -350,6 +366,32 @@ func cutOpenStackClusterStackReleaseVersionFromReleaseTag(releaseTag string) (st
return fmt.Sprintf("%s-%s-%s-%s", v[0], v[1], v[2], v[3]), nil
}

func (r *OpenStackClusterStackReleaseReconciler) getCloudNameFromSecret(ctx context.Context, secretNamespace, secretName string) (string, error) {
var cloudName string
emptyCloudName := ""
defaultCloudName := "openstack"

secret := &corev1.Secret{}
err := r.Get(ctx, types.NamespacedName{
Namespace: secretNamespace,
Name: secretName,
}, secret)
if err != nil {
return emptyCloudName, fmt.Errorf("failed to get secret %s in namespace %s: %w", secretName, secretNamespace, err)
}

content, ok := secret.Data[cloudNameSecretKey]
if !ok {
return defaultCloudName, nil
}

if err := yaml.Unmarshal(content, &cloudName); err != nil {
return emptyCloudName, fmt.Errorf("failed to unmarshal cloudName stored in secret %s: %w", secretName, err)
}

return cloudName, nil
}

// SetupWithManager sets up the controller with the Manager.
func (r *OpenStackClusterStackReleaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
80 changes: 70 additions & 10 deletions internal/controller/openstackclusterstackrelease_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ func TestGenerateOwnerReference(t *testing.T) {
UID: "fb686e33-01a6-42c9-a210-2c26ec8cb331",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "openstack",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand Down Expand Up @@ -103,7 +102,6 @@ func TestMatchOwnerReference(t *testing.T) {
Name: "openstack-ferrol-1-27-v1",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "openstack",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret1",
Expand All @@ -122,7 +120,6 @@ func TestMatchOwnerReference(t *testing.T) {
Name: "openstack-ferrol-1-27-v2",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "openstack",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret2",
Expand Down Expand Up @@ -245,7 +242,6 @@ func TestGetOwnedOpenStackNodeImageReleases(t *testing.T) {
Namespace: "test-namespace",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "test-cloudname",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand Down Expand Up @@ -290,6 +286,8 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
scheme := runtime.NewScheme()
err := apiv1alpha1.AddToScheme(scheme)
assert.NoError(t, err)
err = corev1.AddToScheme(scheme)
assert.NoError(t, err)
client := fake.NewClientBuilder().WithScheme(scheme).Build()

openstackclusterstackrelease := &apiv1alpha1.OpenStackClusterStackRelease{
Expand All @@ -302,7 +300,6 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
Namespace: "test-namespace",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "test-cloudname",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand All @@ -328,11 +325,26 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
UID: openstackclusterstackrelease.UID,
}

secretName := "supersecret"
secretNamespace := "test-namespace"
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: secretNamespace,
},
Type: corev1.SecretTypeOpaque,
}
err = client.Create(context.TODO(), secret)
assert.NoError(t, err)

r := &OpenStackClusterStackReleaseReconciler{
Client: client,
}

err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", openStackNodeImage, ownerRef)
cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName)
assert.NoError(t, err)

err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", cloudName, openStackNodeImage, ownerRef)

assert.NoError(t, err)
osnir := &apiv1alpha1.OpenStackNodeImageRelease{}
Expand All @@ -346,7 +358,7 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
APIVersion: apiv1alpha1.GroupVersion.String(),
},
Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{
CloudName: "test-cloudname",
CloudName: "openstack",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand Down Expand Up @@ -388,7 +400,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) {
Namespace: "test-namespace",
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "test-cloudname",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand Down Expand Up @@ -458,7 +469,7 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, ownerRef.UID, osnir.OwnerReferences[0].UID)

err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", openStackNodeImage, newOwnerRef)
err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", "test-cloud-name", openStackNodeImage, newOwnerRef)
assert.NoError(t, err)

err = client.Get(context.TODO(), types.NamespacedName{Name: "test-update-osnir", Namespace: "test-namespace"}, osnir)
Expand All @@ -473,6 +484,56 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) {
assert.NoError(t, err)
}

func TestGetCloudNameFromSecret(t *testing.T) {
client := fake.NewClientBuilder().Build()

secretName := "supersecret"
secretNamespace := "test-namespace"
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: secretNamespace,
},
Type: corev1.SecretTypeOpaque,
}
err := client.Create(context.TODO(), secret)
assert.NoError(t, err)

r := &OpenStackClusterStackReleaseReconciler{
Client: client,
}

cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName)
expectedCloudName := "openstack"

assert.NoError(t, err)
assert.Equal(t, expectedCloudName, cloudName)

err = client.Delete(context.TODO(), secret)
assert.NoError(t, err)
}

func TestGetCloudNameFromSecretNotFound(t *testing.T) {
client := fake.NewClientBuilder().Build()

r := &OpenStackClusterStackReleaseReconciler{
Client: client,
}

secretName := "nonexistent-secret"
secretNamespace := "nonexistent-namespace"
expectedError := "secrets \"nonexistent-secret\" not found"

cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName)

expectedErrorMessage := fmt.Sprintf("failed to get secret %s in namespace %s: %v", secretName, secretNamespace, expectedError)

assert.Error(t, err)
assert.True(t, apierrors.IsNotFound(err))
assert.Equal(t, "", cloudName)
assert.EqualError(t, err, expectedErrorMessage)
}

var _ = Describe("OpenStackClusterStackRelease controller", func() {
Context("OpenStackClusterStackRelease controller test", func() {
const openstackclusterstackreleasename = "test-ocsr"
Expand Down Expand Up @@ -510,7 +571,6 @@ var _ = Describe("OpenStackClusterStackRelease controller", func() {
Namespace: namespace.Name,
},
Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
CloudName: "openstack",
IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
Kind: "Secret",
Name: "supersecret",
Expand Down
Loading

0 comments on commit 284ff65

Please sign in to comment.