Skip to content

Commit

Permalink
move etcd and image config into the common func as well, fixes bug in…
Browse files Browse the repository at this point in the history 
… the shard reconcile which accidentally used the rootshard's etcd tls secret ref

On-behalf-of: @SAP christoph.mewes@sap.com
  • Loading branch information
@xrstf
xrstf committed Feb 4, 2025
1 parent 2981f0c commit 8f16a2b
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 82 deletions.
30 changes: 0 additions & 30 deletions internal/resources/rootshard/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package rootshard

import (
"fmt"
"strings"

"k8c.io/reconciler/pkg/reconciling"

Expand Down Expand Up @@ -79,23 +78,8 @@ func DeploymentReconciler(rootShard *operatorv1alpha1.RootShard) reconciling.Nam
MountPath: getCAMountPath(operatorv1alpha1.RootCA),
}}

image, _ := resources.GetImageSettings(rootShard.Spec.Image)
args := getArgs(rootShard)

if rootShard.Spec.Etcd.TLSConfig != nil {
secretMounts = append(secretMounts, utils.SecretMount{
VolumeName: "etcd-client-cert",
SecretName: rootShard.Spec.Etcd.TLSConfig.SecretRef.Name,
MountPath: "/etc/etcd/tls",
})

args = append(args,
"--etcd-certfile=/etc/etcd/tls/tls.crt",
"--etcd-keyfile=/etc/etcd/tls/tls.key",
"--etcd-cafile=/etc/etcd/tls/ca.crt",
)
}

for _, cert := range []operatorv1alpha1.Certificate{
// requires server CA and the logical-cluster-admin cert to be mounted
operatorv1alpha1.LogicalClusterAdminCertificate,
Expand Down Expand Up @@ -146,7 +130,6 @@ func DeploymentReconciler(rootShard *operatorv1alpha1.RootShard) reconciling.Nam

dep.Spec.Template.Spec.Containers = []corev1.Container{{
Name: ServerContainerName,
Image: image,
Command: []string{"/kcp", "start"},
Args: args,
VolumeMounts: volumeMounts,
Expand All @@ -158,16 +141,6 @@ func DeploymentReconciler(rootShard *operatorv1alpha1.RootShard) reconciling.Nam
}}
dep.Spec.Template.Spec.Volumes = volumes

// explicitly set the replicas if it is configured in the RootShard
// object or if the existing Deployment object doesn't have replicas
// configured. This will allow a HPA to interact with the replica
// count.
if rootShard.Spec.Replicas != nil {
dep.Spec.Replicas = rootShard.Spec.Replicas
} else if dep.Spec.Replicas == nil {
dep.Spec.Replicas = ptr.To[int32](2)
}

dep, err := utils.ApplyCommonShardConfig(dep, &rootShard.Spec.CommonShardSpec)
if err != nil {
return nil, fmt.Errorf("failed to shard configuration: %w", err)
Expand Down Expand Up @@ -196,9 +169,6 @@ func getArgs(rootShard *operatorv1alpha1.RootShard) []string {
fmt.Sprintf("--service-account-key-file=%s/tls.crt", getCertificateMountPath(operatorv1alpha1.ServiceAccountCertificate)),
fmt.Sprintf("--service-account-private-key-file=%s/tls.key", getCertificateMountPath(operatorv1alpha1.ServiceAccountCertificate)),

// Etcd client configuration.
fmt.Sprintf("--etcd-servers=%s", strings.Join(rootShard.Spec.Etcd.Endpoints, ",")),

// General shard configuration.
fmt.Sprintf("--shard-base-url=%s", resources.GetRootShardBaseURL(rootShard)),
fmt.Sprintf("--shard-external-url=https://%s:%d", rootShard.Spec.External.Hostname, rootShard.Spec.External.Port),
Expand Down
30 changes: 0 additions & 30 deletions internal/resources/shard/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package shard

import (
"fmt"
"strings"

"k8c.io/reconciler/pkg/reconciling"

Expand Down Expand Up @@ -78,23 +77,8 @@ func DeploymentReconciler(shard *operatorv1alpha1.Shard, rootShard *operatorv1al
MountPath: getCAMountPath(operatorv1alpha1.RootCA),
}}

image, _ := resources.GetImageSettings(shard.Spec.Image)
args := getArgs(shard, rootShard)

if shard.Spec.Etcd.TLSConfig != nil {
secretMounts = append(secretMounts, utils.SecretMount{
VolumeName: "etcd-client-cert",
SecretName: rootShard.Spec.Etcd.TLSConfig.SecretRef.Name,
MountPath: "/etc/etcd/tls",
})

args = append(args,
"--etcd-certfile=/etc/etcd/tls/tls.crt",
"--etcd-keyfile=/etc/etcd/tls/tls.key",
"--etcd-cafile=/etc/etcd/tls/ca.crt",
)
}

for _, cert := range []operatorv1alpha1.Certificate{
// requires server CA and the shard client cert to be mounted
operatorv1alpha1.ClientCertificate,
Expand Down Expand Up @@ -143,7 +127,6 @@ func DeploymentReconciler(shard *operatorv1alpha1.Shard, rootShard *operatorv1al

dep.Spec.Template.Spec.Containers = []corev1.Container{{
Name: ServerContainerName,
Image: image,
Command: []string{"/kcp", "start"},
Args: args,
VolumeMounts: volumeMounts,
Expand All @@ -155,16 +138,6 @@ func DeploymentReconciler(shard *operatorv1alpha1.Shard, rootShard *operatorv1al
}}
dep.Spec.Template.Spec.Volumes = volumes

// explicitly set the replicas if it is configured in the RootShard
// object or if the existing Deployment object doesn't have replicas
// configured. This will allow a HPA to interact with the replica
// count.
if shard.Spec.Replicas != nil {
dep.Spec.Replicas = shard.Spec.Replicas
} else if dep.Spec.Replicas == nil {
dep.Spec.Replicas = ptr.To[int32](2)
}

dep, err := utils.ApplyCommonShardConfig(dep, &shard.Spec.CommonShardSpec)
if err != nil {
return nil, fmt.Errorf("failed to shard configuration: %w", err)
Expand Down Expand Up @@ -195,9 +168,6 @@ func getArgs(shard *operatorv1alpha1.Shard, rootShard *operatorv1alpha1.RootShar
fmt.Sprintf("--shard-client-key-file=%s/tls.crt", getCertificateMountPath(operatorv1alpha1.ClientCertificate)),
fmt.Sprintf("--shard-client-cert-file=%s/tls.key", getCertificateMountPath(operatorv1alpha1.ClientCertificate)),

// Etcd client configuration.
fmt.Sprintf("--etcd-servers=%s", strings.Join(shard.Spec.Etcd.Endpoints, ",")),

// General shard configuration.
fmt.Sprintf("--shard-name=%s", shard.Name),
fmt.Sprintf("--external-hostname=%s", resources.GetShardBaseHost(shard)),
Expand Down
11 changes: 3 additions & 8 deletions internal/resources/utils/audit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package utils

import (
"errors"
"fmt"

appsv1 "k8s.io/api/apps/v1"
Expand All @@ -26,16 +25,12 @@ import (
operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
)

func applyAuditConfiguration(deployment *appsv1.Deployment, config *operatorv1alpha1.AuditSpec) (*appsv1.Deployment, error) {
if len(deployment.Spec.Template.Spec.Containers) == 0 {
return deployment, errors.New("Deployment does not contain any containers")
}

func applyAuditConfiguration(deployment *appsv1.Deployment, config *operatorv1alpha1.AuditSpec) *appsv1.Deployment {
if config == nil || config.Webhook == nil {
return deployment, nil
return deployment
}

return applyAuditWebhookConfiguration(deployment, *config.Webhook), nil
return applyAuditWebhookConfiguration(deployment, *config.Webhook)
}

func applyAuditWebhookConfiguration(deployment *appsv1.Deployment, config operatorv1alpha1.AuditWebhookSpec) *appsv1.Deployment {
Expand Down
11 changes: 3 additions & 8 deletions internal/resources/utils/authorization.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package utils

import (
"errors"
"fmt"
"strings"

Expand All @@ -27,16 +26,12 @@ import (
operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
)

func applyAuthorizationConfiguration(deployment *appsv1.Deployment, config *operatorv1alpha1.AuthorizationSpec) (*appsv1.Deployment, error) {
if len(deployment.Spec.Template.Spec.Containers) == 0 {
return deployment, errors.New("Deployment does not contain any containers")
}

func applyAuthorizationConfiguration(deployment *appsv1.Deployment, config *operatorv1alpha1.AuthorizationSpec) *appsv1.Deployment {
if config == nil || config.Webhook == nil {
return deployment, nil
return deployment
}

return applyAuthorizationWebhookConfiguration(deployment, *config.Webhook), nil
return applyAuthorizationWebhookConfiguration(deployment, *config.Webhook)
}

func applyAuthorizationWebhookConfiguration(deployment *appsv1.Deployment, config operatorv1alpha1.AuthorizationWebhookSpec) *appsv1.Deployment {
Expand Down
69 changes: 63 additions & 6 deletions internal/resources/utils/shard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,80 @@ limitations under the License.
package utils

import (
"errors"
"fmt"
"strings"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"

"github.com/kcp-dev/kcp-operator/internal/resources"
operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
)

func ApplyCommonShardConfig(deployment *appsv1.Deployment, spec *operatorv1alpha1.CommonShardSpec) (*appsv1.Deployment, error) {
deployment, err := applyAuditConfiguration(deployment, spec.Audit)
if err != nil {
return nil, fmt.Errorf("failed to apply audit configuration: %w", err)
if len(deployment.Spec.Template.Spec.Containers) == 0 {
return deployment, errors.New("Deployment does not contain any containers")
}

deployment, err = applyAuthorizationConfiguration(deployment, spec.Authorization)
if err != nil {
return nil, fmt.Errorf("failed to apply authorization configuration: %w", err)
// explicitly set the replicas if it is configured in the RootShard
// object or if the existing Deployment object doesn't have replicas
// configured. This will allow a HPA to interact with the replica
// count.
if spec.Replicas != nil {
deployment.Spec.Replicas = spec.Replicas
} else if deployment.Spec.Replicas == nil {
deployment.Spec.Replicas = ptr.To[int32](2)
}

// set container image
image, _ := resources.GetImageSettings(spec.Image)
deployment.Spec.Template.Spec.Containers[0].Image = image

deployment = applyEtcdConfiguration(deployment, spec.Etcd)
deployment = applyAuditConfiguration(deployment, spec.Audit)
deployment = applyAuthorizationConfiguration(deployment, spec.Authorization)

return deployment, nil
}

func applyEtcdConfiguration(deployment *appsv1.Deployment, config operatorv1alpha1.EtcdConfig) *appsv1.Deployment {
podSpec := deployment.Spec.Template.Spec

podSpec.Containers[0].Args = append(
podSpec.Containers[0].Args,
fmt.Sprintf("--etcd-servers=%s", strings.Join(config.Endpoints, ",")),
)

if config.TLSConfig != nil {
volumeName := "etcd-client-cert"
mountPath := "/etc/etcd/tls"

podSpec.Containers[0].VolumeMounts = append(podSpec.Containers[0].VolumeMounts, corev1.VolumeMount{
Name: volumeName,
ReadOnly: true,
MountPath: mountPath,
})

deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, corev1.Volume{
Name: volumeName,
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: config.TLSConfig.SecretRef.Name,
},
},
})

podSpec.Containers[0].Args = append(
podSpec.Containers[0].Args,
fmt.Sprintf("--etcd-certfile=%s/tls.crt", mountPath),
fmt.Sprintf("--etcd-keyfile=%s/tls.key", mountPath),
fmt.Sprintf("--etcd-cafile=%s/ca.crt", mountPath),
)
}

deployment.Spec.Template.Spec = podSpec

return deployment
}

0 comments on commit 8f16a2b

Please sign in to comment.