Skip to content

Commit

Permalink
fix: use bootstrap data secret names
Browse files Browse the repository at this point in the history
Fixes #38

Fixes:

* bootstrap provider now uses proper passed in bootstrap data secret
name instead of hardcoding it in the code

* try to remove finalizer early, so that talosconfig can be deleted even
if the owner is gone

Tests:

* existing tests refactored for readability

* added test for full cluster config

* added test for custom cluster settings

* added test for config patches

* talosVersion field is now passed to trigger proper generation

* addressed test stability by waiting properly for namespace teardown

Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
  • Loading branch information
smira committed Sep 20, 2021
1 parent 6bff239 commit f91b032
Show file tree
Hide file tree
Showing 9 changed files with 373 additions and 144 deletions.
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ COPY --from=generate-build /src/api /api

FROM build AS integration-test-build
ENV CGO_ENABLED 1
ARG GO_LDFLAGS="-linkmode=external -extldflags '-static'"
ARG TALOS_VERSION
ARG GO_LDFLAGS="-linkmode=external -extldflags '-static' -X github.com/talos-systems/cluster-api-bootstrap-provider-talos/internal/integration.TalosVersion=${TALOS_VERSION}"
RUN --mount=type=cache,target=/.cache go test -race -ldflags "${GO_LDFLAGS}" -coverpkg=./... -v -c ./internal/integration

FROM scratch AS integration-test
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ COMMON_ARGS += --build-arg=PKGS=$(PKGS)
COMMON_ARGS += --build-arg=TOOLS=$(TOOLS)
COMMON_ARGS += --build-arg=CONTROLLER_GEN_VERSION=$(CONTROLLER_GEN_VERSION)
COMMON_ARGS += --build-arg=CONVERSION_GEN_VERSION=$(CONVERSION_GEN_VERSION)
COMMON_ARGS += --build-arg=TALOS_VERSION=$(TALOS_VERSION)

all: manifests container

Expand Down
9 changes: 7 additions & 2 deletions controllers/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package controllers

import (
"context"
"fmt"

"github.com/talos-systems/crypto/x509"
"github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1/generate"
Expand Down Expand Up @@ -112,14 +113,18 @@ func (r *TalosConfigReconciler) writeBootstrapData(ctx context.Context, scope *T
// Create ca secret only if it doesn't already exist
ownerName := scope.ConfigOwner.GetName()

if scope.ConfigOwner.DataSecretName() == nil {
return fmt.Errorf("config owner data secret name is nil")
}

r.Log.Info("handling bootstrap data for ", "owner", ownerName)

_, err := r.fetchSecret(ctx, scope.Config, ownerName+"-bootstrap-data")
_, err := r.fetchSecret(ctx, scope.Config, *scope.ConfigOwner.DataSecretName())
if k8serrors.IsNotFound(err) {
certSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: scope.Config.Namespace,
Name: ownerName + "-bootstrap-data",
Name: *scope.ConfigOwner.DataSecretName(),
Labels: map[string]string{
capiv1.ClusterLabelName: scope.Cluster.Name,
},
Expand Down
41 changes: 20 additions & 21 deletions controllers/talosconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/pointer"
capiv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
bsutil "sigs.k8s.io/cluster-api/bootstrap/util"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1alpha3"
Expand Down Expand Up @@ -145,25 +144,6 @@ func (r *TalosConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, rerr
return ctrl.Result{}, err
}

// Look up the resource that owns this talosconfig if there is one
owner, err := bsutil.GetConfigOwner(ctx, r.Client, config)
if err != nil {
log.Error(err, "could not get owner resource")
return ctrl.Result{}, err
}
if owner == nil {
log.Info("Waiting for OwnerRef on the talosconfig")
return ctrl.Result{}, errors.New("no owner ref")
}
log = log.WithName(fmt.Sprintf("owner-name=%s", owner.GetName()))

// Lookup the cluster the machine is associated with
cluster, err := util.GetClusterByName(ctx, r.Client, owner.GetNamespace(), owner.ClusterName())
if err != nil {
log.Error(err, "could not get cluster by machine metadata")
return ctrl.Result{}, err
}

// Initialize the patch helper
patchHelper, err := patch.NewHelper(config, r.Client)
if err != nil {
Expand All @@ -187,6 +167,25 @@ func (r *TalosConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, rerr
return r.reconcileDelete(ctx, config)
}

// Look up the resource that owns this talosconfig if there is one
owner, err := bsutil.GetConfigOwner(ctx, r.Client, config)
if err != nil {
log.Error(err, "could not get owner resource")
return ctrl.Result{}, err
}
if owner == nil {
log.Info("Waiting for OwnerRef on the talosconfig")
return ctrl.Result{}, errors.New("no owner ref")
}
log = log.WithName(fmt.Sprintf("owner-name=%s", owner.GetName()))

// Lookup the cluster the machine is associated with
cluster, err := util.GetClusterByName(ctx, r.Client, owner.GetNamespace(), owner.ClusterName())
if err != nil {
log.Error(err, "could not get cluster by machine metadata")
return ctrl.Result{}, err
}

// bail super early if it's already ready
if config.Status.Ready {
log.Info("ignoring an already ready config")
Expand Down Expand Up @@ -275,7 +274,7 @@ func (r *TalosConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, rerr
return ctrl.Result{}, err
}

config.Status.DataSecretName = pointer.StringPtr(tcScope.ConfigOwner.GetName() + "-bootstrap-data")
config.Status.DataSecretName = tcScope.ConfigOwner.DataSecretName()
config.Status.TalosConfig = retData.TalosConfig
config.Status.Ready = true

Expand Down
148 changes: 148 additions & 0 deletions internal/integration/assertions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

package integration

import (
"context"
"testing"

"github.com/AlekSi/pointer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
bootstrapv1alpha3 "github.com/talos-systems/cluster-api-bootstrap-provider-talos/api/v1alpha3"
talosclientconfig "github.com/talos-systems/talos/pkg/machinery/client/config"
machineconfig "github.com/talos-systems/talos/pkg/machinery/config"
"github.com/talos-systems/talos/pkg/machinery/config/configloader"
"github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1/generate"
"gopkg.in/yaml.v2"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
capiv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// assertClientConfig checks that Talos client config as part of TalosConfig resource is valid.
func assertClientConfig(t *testing.T, talosConfig *bootstrapv1alpha3.TalosConfig) {
t.Helper()

clientConfig, err := talosclientconfig.FromString(talosConfig.Status.TalosConfig)
require.NoError(t, err)
validateClientConfig(t, clientConfig)
}

// assertMachineConfiguration checks that generated bootstrap data is a valid Talos machine configuration.
func assertMachineConfiguration(ctx context.Context, t *testing.T, c client.Client, talosConfig *bootstrapv1alpha3.TalosConfig) machineconfig.Provider {
var bootstrapDataSecret corev1.Secret

key := types.NamespacedName{
Namespace: talosConfig.Namespace,
Name: pointer.GetString(talosConfig.Status.DataSecretName),
}
require.NoError(t, c.Get(ctx, key, &bootstrapDataSecret))

assert.Len(t, bootstrapDataSecret.Data, 1)

provider, err := configloader.NewFromBytes(bootstrapDataSecret.Data["value"])
require.NoError(t, err)

_, err = provider.Validate(runtimeMode{false}, machineconfig.WithStrict())
assert.NoError(t, err)

return provider
}

// assertClusterCA checks that generated cluster CA secret matches secrets in machine config (machine config from controlplane node required).
func assertClusterCA(ctx context.Context, t *testing.T, c client.Client, cluster *capiv1.Cluster, provider machineconfig.Provider) {
var caSecret corev1.Secret

key := types.NamespacedName{
Namespace: cluster.Namespace,
Name: cluster.Name + "-ca",
}
require.NoError(t, c.Get(ctx, key, &caSecret))

assert.Len(t, caSecret.Data, 2)
assert.Equal(t, corev1.SecretTypeOpaque, caSecret.Type) // TODO why not SecretTypeTLS?

assert.NotEmpty(t, caSecret.Data[corev1.TLSCertKey])
assert.NotEmpty(t, caSecret.Data[corev1.TLSPrivateKeyKey])

assert.Equal(t, provider.Cluster().CA().Crt, caSecret.Data[corev1.TLSCertKey])
assert.Equal(t, provider.Cluster().CA().Key, caSecret.Data[corev1.TLSPrivateKeyKey])
}

// assertControllerSecret checks that persisted controller secret (used to bootstrap more machines with same secrets) maches generated controlplane config.
func assertControllerSecret(ctx context.Context, t *testing.T, c client.Client, cluster *capiv1.Cluster, provider machineconfig.Provider) {
var talosSecret corev1.Secret
key := types.NamespacedName{
Namespace: cluster.Namespace,
Name: cluster.Name + "-talos",
}
require.NoError(t, c.Get(ctx, key, &talosSecret))

assert.Len(t, talosSecret.Data, 3)
assert.NotEmpty(t, talosSecret.Data["certs"])
assert.NotEmpty(t, talosSecret.Data["kubeSecrets"])
assert.NotEmpty(t, talosSecret.Data["trustdInfo"])

// cross-checks
secretsBundle := generate.NewSecretsBundleFromConfig(generate.NewClock(), provider)

var certs generate.Certs
require.NoError(t, yaml.Unmarshal(talosSecret.Data["certs"], &certs))
assert.NotEmpty(t, certs.Admin)
certs.Admin = nil
assert.Equal(t, secretsBundle.Certs, &certs)

var kubeSecrets generate.Secrets
require.NoError(t, yaml.Unmarshal(talosSecret.Data["kubeSecrets"], &kubeSecrets))
assert.Equal(t, secretsBundle.Secrets, &kubeSecrets)

var trustdInfo generate.TrustdInfo
require.NoError(t, yaml.Unmarshal(talosSecret.Data["trustdInfo"], &trustdInfo))
assert.Equal(t, secretsBundle.TrustdInfo, &trustdInfo)
}

// assertSameMachineConfigSecrets checks that control plane configs share same set of secrets.
func assertSameMachineConfigSecrets(ctx context.Context, t *testing.T, c client.Client, talosConfigs ...*bootstrapv1alpha3.TalosConfig) {
providers := make([]machineconfig.Provider, len(talosConfigs))

for i := range providers {
providers[i] = assertMachineConfiguration(ctx, t, c, talosConfigs[i])
}

secretsBundle0 := generate.NewSecretsBundleFromConfig(generate.NewClock(), providers[0])

for _, provider := range providers[1:] {
assert.Equal(t, secretsBundle0, generate.NewSecretsBundleFromConfig(generate.NewClock(), provider))
}
}

// assertCompatibleMachineConfigs checks that configs share same set of core secrets so that nodes can build a cluster.
func assertCompatibleMachineConfigs(ctx context.Context, t *testing.T, c client.Client, talosConfigs ...*bootstrapv1alpha3.TalosConfig) {
providers := make([]machineconfig.Provider, len(talosConfigs))

for i := range providers {
providers[i] = assertMachineConfiguration(ctx, t, c, talosConfigs[i])
}

checks := []func(p machineconfig.Provider) interface{}{
func(p machineconfig.Provider) interface{} { return p.Machine().Security().Token() },
// TODO: uncomment me with Talos 0.12: func(p machineconfig.Provider) interface{} { return p.Cluster().ID() },
// TODO: uncomment me with Talos 0.12: func(p machineconfig.Provider) interface{} { return p.Cluster().Secret() },
func(p machineconfig.Provider) interface{} { return p.Cluster().Endpoint().String() },
func(p machineconfig.Provider) interface{} { return p.Cluster().Token().ID() },
func(p machineconfig.Provider) interface{} { return p.Cluster().Token().Secret() },
func(p machineconfig.Provider) interface{} { return p.Cluster().CA().Crt },
}

for _, check := range checks {
value0 := check(providers[0])

for _, provider := range providers[1:] {
assert.Equal(t, value0, check(provider))
}
}
}
8 changes: 8 additions & 0 deletions internal/integration/constants_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

package integration

// TalosVersion is set by the build process.
var TalosVersion string
45 changes: 22 additions & 23 deletions internal/integration/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package integration

import (
"context"
"crypto/x509"
"encoding/pem"
"flag"
"fmt"
"os"
Expand All @@ -16,6 +14,7 @@ import (
"testing"
"time"

"github.com/AlekSi/pointer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
talosclient "github.com/talos-systems/talos/pkg/machinery/client"
Expand All @@ -25,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
capiv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
bsutil "sigs.k8s.io/cluster-api/bootstrap/util"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

Expand Down Expand Up @@ -61,22 +61,27 @@ func generateName(t *testing.T, kind string) string {
}

// createCluster creates a Cluster with "ready" infrastructure.
func createCluster(ctx context.Context, t *testing.T, c client.Client, namespaceName string) *capiv1.Cluster {
func createCluster(ctx context.Context, t *testing.T, c client.Client, namespaceName string, spec *capiv1.ClusterSpec) *capiv1.Cluster {
t.Helper()

clusterName := generateName(t, "cluster")
cluster := &capiv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespaceName,
Name: clusterName,
},
Spec: capiv1.ClusterSpec{

if spec == nil {
spec = &capiv1.ClusterSpec{
ClusterNetwork: &capiv1.ClusterNetwork{},
ControlPlaneEndpoint: capiv1.APIEndpoint{
Host: clusterName + ".host",
Port: 12345,
},
}
}

cluster := &capiv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespaceName,
Name: clusterName,
},
Spec: *spec,
}

require.NoError(t, c.Create(ctx, cluster), "can't create a cluster")
Expand All @@ -92,7 +97,7 @@ func createMachine(ctx context.Context, t *testing.T, c client.Client, cluster *
t.Helper()

machineName := generateName(t, "machine")
dataSecretName := "my-test-secret"
dataSecretName := fmt.Sprintf("%s-bootstrap-data", machineName)
machine := &capiv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Namespace: cluster.Namespace,
Expand All @@ -101,7 +106,7 @@ func createMachine(ctx context.Context, t *testing.T, c client.Client, cluster *
Spec: capiv1.MachineSpec{
ClusterName: cluster.Name,
Bootstrap: capiv1.Bootstrap{
DataSecretName: &dataSecretName, // TODO
DataSecretName: pointer.ToString(dataSecretName),
},
},
}
Expand All @@ -114,7 +119,7 @@ func createMachine(ctx context.Context, t *testing.T, c client.Client, cluster *
}

// createTalosConfig creates a TalosConfig owned by the Machine.
func createTalosConfig(ctx context.Context, t *testing.T, c client.Client, machine *capiv1.Machine) *bootstrapv1alpha3.TalosConfig {
func createTalosConfig(ctx context.Context, t *testing.T, c client.Client, machine *capiv1.Machine, spec bootstrapv1alpha3.TalosConfigSpec) *bootstrapv1alpha3.TalosConfig {
t.Helper()

talosConfigName := generateName(t, "talosconfig")
Expand All @@ -123,9 +128,7 @@ func createTalosConfig(ctx context.Context, t *testing.T, c client.Client, machi
Namespace: machine.Namespace,
Name: talosConfigName,
},
Spec: bootstrapv1alpha3.TalosConfigSpec{
GenerateType: "init",
},
Spec: spec,
}

require.NoError(t, controllerutil.SetOwnerReference(machine, talosConfig, scheme.Scheme))
Expand Down Expand Up @@ -163,16 +166,12 @@ func waitForReady(ctx context.Context, t *testing.T, c client.Client, talosConfi
t.Log("Waiting ...")
sleepCtx(ctx, 3*time.Second)
}
}

// parsePEMCertificate parses PEM-encoded x509 certificate.
func parsePEMCertificate(t *testing.T, b []byte) *x509.Certificate {
block, rest := pem.Decode(b)
assert.Empty(t, rest)
require.NotEmpty(t, block.Bytes)
cert, err := x509.ParseCertificate(block.Bytes)
owner, err := bsutil.GetConfigOwner(ctx, c, talosConfig)
require.NoError(t, err)
return cert

assert.Equal(t, pointer.GetString(owner.DataSecretName()), pointer.GetString(talosConfig.Status.DataSecretName), "%+v", talosConfig)

}

// validateClientConfig validates talosctl configuration.
Expand Down
Loading

0 comments on commit f91b032

Please sign in to comment.