From 89c56c269f6e4bb4ad15707bc3a405a3d5d69354 Mon Sep 17 00:00:00 2001 From: Moritz Johner Date: Mon, 15 Feb 2021 21:51:38 +0100 Subject: [PATCH] feat: status conditions (#25) * feat: implement es ready condition Co-authored-by: Kellin --- .../v1alpha1/externalsecret_types.go | 15 +- .../external-secrets.io_externalsecrets.yaml | 7 + .../externalsecret_controller.go | 15 +- .../externalsecret_controller_test.go | 190 ++++++++++++++++++ pkg/controllers/externalsecret/suite_test.go | 27 ++- pkg/controllers/externalsecret/util.go | 67 ++++++ pkg/provider/fake/fake.go | 2 +- 7 files changed, 313 insertions(+), 10 deletions(-) create mode 100644 pkg/controllers/externalsecret/externalsecret_controller_test.go create mode 100644 pkg/controllers/externalsecret/util.go diff --git a/apis/externalsecrets/v1alpha1/externalsecret_types.go b/apis/externalsecrets/v1alpha1/externalsecret_types.go index 71e3aefd6..378be072f 100644 --- a/apis/externalsecrets/v1alpha1/externalsecret_types.go +++ b/apis/externalsecrets/v1alpha1/externalsecret_types.go @@ -141,19 +141,28 @@ type ExternalSecretStatusCondition struct { LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` } +const ( + // ConditionReasonSecretSynced indicates that the secrets was synced. + ConditionReasonSecretSynced = "SecretSynced" + // ConditionReasonSecretSyncedError indicates that there was an error syncing the secret. + ConditionReasonSecretSyncedError = "SecretSyncedError" +) + type ExternalSecretStatus struct { - // +optional + // +nullable // refreshTime is the time and date the external secret was fetched and // the target secret updated - RefreshTime metav1.Time `json:"refreshTime"` + RefreshTime metav1.Time `json:"refreshTime,omitempty"` // +optional - Conditions []ExternalSecretStatusCondition `json:"conditions"` + Conditions []ExternalSecretStatusCondition `json:"conditions,omitempty"` } // +kubebuilder:object:root=true // ExternalSecret is the Schema for the external-secrets API. +// +kubebuilder:subresource:status +// +kubebuilder:resource:scope=Namespaced,categories={externalsecrets},shortName=es type ExternalSecret struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/external-secrets.io_externalsecrets.yaml b/config/crd/bases/external-secrets.io_externalsecrets.yaml index 7c66311d9..0a8fafcd7 100644 --- a/config/crd/bases/external-secrets.io_externalsecrets.yaml +++ b/config/crd/bases/external-secrets.io_externalsecrets.yaml @@ -8,9 +8,13 @@ metadata: spec: group: external-secrets.io names: + categories: + - externalsecrets kind: ExternalSecret listKind: ExternalSecretList plural: externalsecrets + shortNames: + - es singular: externalsecret scope: Namespaced versions: @@ -152,11 +156,14 @@ spec: description: refreshTime is the time and date the external secret was fetched and the target secret updated format: date-time + nullable: true type: string type: object type: object served: true storage: true + subresources: + status: {} status: acceptedNames: kind: "" diff --git a/pkg/controllers/externalsecret/externalsecret_controller.go b/pkg/controllers/externalsecret/externalsecret_controller.go index 707c9f930..bff631957 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller.go +++ b/pkg/controllers/externalsecret/externalsecret_controller.go @@ -64,7 +64,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: externalSecret.Name, + Name: externalSecret.Spec.Target.Name, Namespace: externalSecret.Namespace, }, } @@ -108,9 +108,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err != nil { log.Error(err, "could not reconcile ExternalSecret") + conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionFalse, esv1alpha1.ConditionReasonSecretSynced, err.Error()) + SetExternalSecretCondition(&externalSecret.Status, *conditionSynced) + err = r.Status().Update(ctx, &externalSecret) + if err != nil { + log.Error(err, "unable to update status") + } return ctrl.Result{RequeueAfter: requeueAfter}, nil } + conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionTrue, esv1alpha1.ConditionReasonSecretSynced, "Secret was synced") + SetExternalSecretCondition(&externalSecret.Status, *conditionSynced) + externalSecret.Status.RefreshTime = metav1.NewTime(time.Now()) + err = r.Status().Update(ctx, &externalSecret) + if err != nil { + log.Error(err, "unable to update status") + } return ctrl.Result{}, nil } diff --git a/pkg/controllers/externalsecret/externalsecret_controller_test.go b/pkg/controllers/externalsecret/externalsecret_controller_test.go new file mode 100644 index 000000000..7f170ebd3 --- /dev/null +++ b/pkg/controllers/externalsecret/externalsecret_controller_test.go @@ -0,0 +1,190 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package externalsecret + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "sigs.k8s.io/controller-runtime/pkg/client" + + esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1" + "github.com/external-secrets/external-secrets/pkg/provider/fake" + "github.com/external-secrets/external-secrets/pkg/provider/schema" +) + +var fakeProvider *fake.Client + +var _ = Describe("ExternalSecret controller", func() { + const ( + ExternalSecretName = "test-es" + ExternalSecretStore = "test-store" + ExternalSecretTargetSecretName = "test-secret" + timeout = time.Second * 5 + interval = time.Millisecond * 250 + ) + + var ExternalSecretNamespace string + + BeforeEach(func() { + var err error + ExternalSecretNamespace, err = CreateNamespace("test-ns", k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient.Create(context.Background(), &esv1alpha1.SecretStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: ExternalSecretStore, + Namespace: ExternalSecretNamespace, + }, + Spec: esv1alpha1.SecretStoreSpec{ + Provider: &esv1alpha1.SecretStoreProvider{ + AWSSM: &esv1alpha1.AWSSMProvider{}, + }, + }, + })).To(Succeed()) + + }) + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: ExternalSecretNamespace, + }, + }, client.PropagationPolicy(metav1.DeletePropagationBackground)), client.GracePeriodSeconds(0)).To(Succeed()) + Expect(k8sClient.Delete(context.Background(), &esv1alpha1.SecretStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: ExternalSecretStore, + Namespace: ExternalSecretNamespace, + }, + }, client.PropagationPolicy(metav1.DeletePropagationBackground)), client.GracePeriodSeconds(0)).To(Succeed()) + }) + + Context("When updating ExternalSecret Status", func() { + It("should set the condition eventually", func() { + By("creating an ExternalSecret") + ctx := context.Background() + es := &esv1alpha1.ExternalSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ExternalSecretName, + Namespace: ExternalSecretNamespace, + }, + Spec: esv1alpha1.ExternalSecretSpec{ + SecretStoreRef: esv1alpha1.SecretStoreRef{ + Name: ExternalSecretStore, + }, + Target: esv1alpha1.ExternalSecretTarget{ + Name: ExternalSecretTargetSecretName, + }, + }, + } + Expect(k8sClient.Create(ctx, es)).Should(Succeed()) + esLookupKey := types.NamespacedName{Name: ExternalSecretName, Namespace: ExternalSecretNamespace} + createdES := &esv1alpha1.ExternalSecret{} + Eventually(func() bool { + err := k8sClient.Get(ctx, esLookupKey, createdES) + if err != nil { + return false + } + cond := GetExternalSecretCondition(createdES.Status, esv1alpha1.ExternalSecretReady) + if cond == nil || cond.Status != v1.ConditionTrue { + return false + } + return true + }, timeout, interval).Should(BeTrue()) + + }) + }) + + Context("When syncing ExternalSecret value", func() { + It("should set the secret value", func() { + By("creating an ExternalSecret") + ctx := context.Background() + const targetProp = "targetProperty" + const secretVal = "someValue" + es := &esv1alpha1.ExternalSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ExternalSecretName, + Namespace: ExternalSecretNamespace, + }, + Spec: esv1alpha1.ExternalSecretSpec{ + SecretStoreRef: esv1alpha1.SecretStoreRef{ + Name: ExternalSecretStore, + }, + Target: esv1alpha1.ExternalSecretTarget{ + Name: ExternalSecretTargetSecretName, + }, + Data: []esv1alpha1.ExternalSecretData{ + { + SecretKey: targetProp, + RemoteRef: esv1alpha1.ExternalSecretDataRemoteRef{ + Key: "barz", + Property: "bang", + }, + }, + }, + }, + } + + fakeProvider.WithGetSecret([]byte(secretVal), nil) + Expect(k8sClient.Create(ctx, es)).Should(Succeed()) + secretLookupKey := types.NamespacedName{ + Name: ExternalSecretTargetSecretName, + Namespace: ExternalSecretNamespace} + syncedSecret := &v1.Secret{} + Eventually(func() bool { + err := k8sClient.Get(ctx, secretLookupKey, syncedSecret) + if err != nil { + return false + } + v := syncedSecret.Data[targetProp] + return string(v) == secretVal + }, timeout, interval).Should(BeTrue()) + + }) + }) +}) + +// CreateNamespace creates a new namespace in the cluster. +func CreateNamespace(baseName string, c client.Client) (string, error) { + genName := fmt.Sprintf("ctrl-test-%v", baseName) + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: genName, + }, + } + var err error + err = wait.Poll(time.Second, 10*time.Second, func() (bool, error) { + err = c.Create(context.Background(), ns) + if err != nil { + return false, nil + } + return true, nil + }) + if err != nil { + return "", err + } + return ns.Name, nil +} + +func init() { + fakeProvider = fake.New() + schema.ForceRegister(fakeProvider, &esv1alpha1.SecretStoreProvider{ + AWSSM: &esv1alpha1.AWSSMProvider{}, + }) +} diff --git a/pkg/controllers/externalsecret/suite_test.go b/pkg/controllers/externalsecret/suite_test.go index 4a00ec46a..12aa81662 100644 --- a/pkg/controllers/externalsecret/suite_test.go +++ b/pkg/controllers/externalsecret/suite_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/envtest/printer" @@ -52,7 +53,7 @@ var _ = BeforeSuite(func(done Done) { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, } var err error @@ -63,14 +64,30 @@ var _ = BeforeSuite(func(done Done) { err = esv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) - err = esv1alpha1.AddToScheme(scheme.Scheme) - Expect(err).NotTo(HaveOccurred()) - // +kubebuilder:scaffold:scheme - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + }) Expect(err).ToNot(HaveOccurred()) + + // do not use k8sManager.GetClient() + // see https://github.com/kubernetes-sigs/controller-runtime/issues/343#issuecomment-469435686 + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(k8sClient).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + + err = (&Reconciler{ + Client: k8sClient, + Scheme: k8sManager.GetScheme(), + Log: ctrl.Log.WithName("controllers").WithName("ExternalSecrets"), + }).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + + go func() { + defer GinkgoRecover() + Expect(k8sManager.Start(ctrl.SetupSignalHandler())).ToNot(HaveOccurred()) + }() close(done) }, 60) diff --git a/pkg/controllers/externalsecret/util.go b/pkg/controllers/externalsecret/util.go new file mode 100644 index 000000000..1510bbca9 --- /dev/null +++ b/pkg/controllers/externalsecret/util.go @@ -0,0 +1,67 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package externalsecret + +import ( + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1" +) + +func NewExternalSecretCondition(condType esv1alpha1.ExternalSecretConditionType, status v1.ConditionStatus, reason, message string) *esv1alpha1.ExternalSecretStatusCondition { + return &esv1alpha1.ExternalSecretStatusCondition{ + Type: condType, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + } +} + +// GetExternalSecretCondition returns the condition with the provided type. +func GetExternalSecretCondition(status esv1alpha1.ExternalSecretStatus, condType esv1alpha1.ExternalSecretConditionType) *esv1alpha1.ExternalSecretStatusCondition { + for i := range status.Conditions { + c := status.Conditions[i] + if c.Type == condType { + return &c + } + } + return nil +} + +// SetExternalSecretCondition updates the external secret to include the provided +// condition. +func SetExternalSecretCondition(status *esv1alpha1.ExternalSecretStatus, condition esv1alpha1.ExternalSecretStatusCondition) { + currentCond := GetExternalSecretCondition(*status, condition.Type) + if currentCond != nil && currentCond.Status == condition.Status && currentCond.Reason == condition.Reason { + return + } + // Do not update lastTransitionTime if the status of the condition doesn't change. + if currentCond != nil && currentCond.Status == condition.Status { + condition.LastTransitionTime = currentCond.LastTransitionTime + } + status.Conditions = append(filterOutCondition(status.Conditions, condition.Type), condition) +} + +func filterOutCondition(conditions []esv1alpha1.ExternalSecretStatusCondition, condType esv1alpha1.ExternalSecretConditionType) []esv1alpha1.ExternalSecretStatusCondition { + newConditions := make([]esv1alpha1.ExternalSecretStatusCondition, 0, len(conditions)) + for _, c := range conditions { + if c.Type == condType { + continue + } + newConditions = append(newConditions, c) + } + return newConditions +} diff --git a/pkg/provider/fake/fake.go b/pkg/provider/fake/fake.go index 7eea2f798..609234efa 100644 --- a/pkg/provider/fake/fake.go +++ b/pkg/provider/fake/fake.go @@ -46,7 +46,7 @@ func New() *Client { } v.NewFn = func(context.Context, esv1alpha1.GenericStore, client.Client, string) (provider.Provider, error) { - return nil, nil + return v, nil } return v