Skip to content

Commit 626e860

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3569 from gman0/cachedresources-identity-fix
CachedResources identity fix and tests
2 parents 1fdaf9d + 793623b commit 626e860

File tree

3 files changed

+347
-56
lines changed

3 files changed

+347
-56
lines changed

‎pkg/reconciler/cache/cachedresources/cachedresources_reconcile.go‎

Lines changed: 9 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ package cachedresources
1818

1919
import (
2020
"context"
21-
"crypto/sha256"
22-
"fmt"
2321
"time"
2422

2523
corev1 "k8s.io/api/core/v1"
@@ -41,7 +39,6 @@ import (
4139
"github.com/kcp-dev/kcp/pkg/tombstone"
4240
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
4341
cachev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/cache/v1alpha1"
44-
"github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions"
4542
)
4643

4744
type reconcileStatus int
@@ -63,11 +60,10 @@ func (c *Controller) reconcile(ctx context.Context, cluster logicalcluster.Name,
6360
reconcilers := []reconciler{
6461
&finalizer{},
6562
&identity{
66-
ensureSecretNamespaceExists: c.ensureSecretNamespaceExists,
67-
getSecret: c.getSecret,
68-
createIdentitySecret: c.createIdentitySecret,
69-
updateOrVerifyIdentitySecretHash: c.updateOrVerifyIdentitySecretHash,
70-
secretNamespace: c.secretNamespace,
63+
ensureSecretNamespaceExists: c.ensureSecretNamespaceExists,
64+
getSecret: c.getSecret,
65+
createIdentitySecret: c.createIdentitySecret,
66+
secretNamespace: c.secretNamespace,
7167
},
7268
&schemaSource{
7369
getLogicalCluster: c.getLogicalCluster,
@@ -208,13 +204,13 @@ func (c *Controller) listSelectedCacheResources(ctx context.Context, cluster log
208204
return resources, nil
209205
}
210206

211-
func (c *Controller) ensureSecretNamespaceExists(ctx context.Context, clusterName logicalcluster.Name) {
207+
func (c *Controller) ensureSecretNamespaceExists(ctx context.Context, clusterName logicalcluster.Name, defaultSecretNamespace string) {
212208
logger := klog.FromContext(ctx)
213209
ctx = klog.NewContext(ctx, logger)
214-
if _, err := c.getNamespace(clusterName, c.secretNamespace); errors.IsNotFound(err) {
210+
if _, err := c.getNamespace(clusterName, defaultSecretNamespace); errors.IsNotFound(err) {
215211
ns := &corev1.Namespace{
216212
ObjectMeta: metav1.ObjectMeta{
217-
Name: c.secretNamespace,
213+
Name: defaultSecretNamespace,
218214
Annotations: map[string]string{logicalcluster.AnnotationKey: clusterName.String()},
219215
},
220216
}
@@ -226,8 +222,8 @@ func (c *Controller) ensureSecretNamespaceExists(ctx context.Context, clusterNam
226222
}
227223
}
228224

229-
func (c *Controller) createIdentitySecret(ctx context.Context, clusterName logicalcluster.Path, apiExportName string) error {
230-
secret, err := GenerateIdentitySecret(ctx, c.secretNamespace, apiExportName)
225+
func (c *Controller) createIdentitySecret(ctx context.Context, clusterName logicalcluster.Path, defaultSecretNamespace, cachedResourceName string) error {
226+
secret, err := GenerateIdentitySecret(ctx, defaultSecretNamespace, cachedResourceName)
231227
if err != nil {
232228
return err
233229
}
@@ -239,42 +235,6 @@ func (c *Controller) createIdentitySecret(ctx context.Context, clusterName logic
239235
return c.createSecret(ctx, clusterName, secret)
240236
}
241237

242-
func (c *Controller) updateOrVerifyIdentitySecretHash(ctx context.Context, clusterName logicalcluster.Name, cachedResource *cachev1alpha1.CachedResource) error {
243-
secret, err := c.getSecret(ctx, clusterName, c.secretNamespace, cachedResource.Name)
244-
if err != nil {
245-
return err
246-
}
247-
248-
hash, err := IdentityHash(secret)
249-
if err != nil {
250-
return err
251-
}
252-
253-
if cachedResource.Status.IdentityHash == "" {
254-
cachedResource.Status.IdentityHash = hash
255-
}
256-
257-
if cachedResource.Status.IdentityHash != hash {
258-
return fmt.Errorf("hash mismatch: identity secret hash %q must match status.identityHash %q", hash, cachedResource.Status.IdentityHash)
259-
}
260-
261-
conditions.MarkTrue(cachedResource, cachev1alpha1.CachedResourceIdentityValid)
262-
263-
return nil
264-
}
265-
266-
// TODO: This is copy from apiexport controller. We should move it to a shared location.
267-
func IdentityHash(secret *corev1.Secret) (string, error) {
268-
key := secret.Data[apisv1alpha1.SecretKeyAPIExportIdentity]
269-
if len(key) == 0 {
270-
return "", fmt.Errorf("secret is missing data.%s", apisv1alpha1.SecretKeyAPIExportIdentity)
271-
}
272-
273-
hashBytes := sha256.Sum256(key)
274-
hash := fmt.Sprintf("%x", hashBytes)
275-
return hash, nil
276-
}
277-
278238
// TODO: This is copy from apiexport controller. We should move it to a shared location.
279239
func GenerateIdentitySecret(ctx context.Context, ns string, name string) (*corev1.Secret, error) {
280240
logger := klog.FromContext(ctx)

‎pkg/reconciler/cache/cachedresources/cachedresources_reconcile_identity.go‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,25 @@ package cachedresources
1818

1919
import (
2020
"context"
21+
"crypto/sha256"
2122
"fmt"
2223

2324
corev1 "k8s.io/api/core/v1"
2425
"k8s.io/apimachinery/pkg/api/errors"
2526

2627
"github.com/kcp-dev/logicalcluster/v3"
2728

29+
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
2830
cachev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/cache/v1alpha1"
2931
conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1"
3032
"github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions"
3133
)
3234

3335
// identity creates identity secret for the published resource and computes hash of the secret.
3436
type identity struct {
35-
ensureSecretNamespaceExists func(ctx context.Context, clusterName logicalcluster.Name)
36-
getSecret func(ctx context.Context, clusterName logicalcluster.Name, namespace, name string) (*corev1.Secret, error)
37-
createIdentitySecret func(ctx context.Context, clusterName logicalcluster.Path, name string) error
38-
updateOrVerifyIdentitySecretHash func(ctx context.Context, clusterName logicalcluster.Name, CachedResource *cachev1alpha1.CachedResource) error
37+
ensureSecretNamespaceExists func(ctx context.Context, clusterName logicalcluster.Name, defaultSecretNamespace string)
38+
getSecret func(ctx context.Context, clusterName logicalcluster.Name, namespace, name string) (*corev1.Secret, error)
39+
createIdentitySecret func(ctx context.Context, clusterName logicalcluster.Path, defaultSecretNamespace, name string) error
3940

4041
secretNamespace string
4142
}
@@ -52,7 +53,7 @@ func (r *identity) reconcile(ctx context.Context, cachedResource *cachev1alpha1.
5253
clusterName := logicalcluster.From(cachedResource)
5354

5455
if identity.SecretRef == nil {
55-
r.ensureSecretNamespaceExists(ctx, clusterName)
56+
r.ensureSecretNamespaceExists(ctx, clusterName, r.secretNamespace)
5657

5758
// See if the generated secret already exists (for whatever reason)
5859
_, err := r.getSecret(ctx, clusterName, r.secretNamespace, cachedResource.Name)
@@ -64,7 +65,7 @@ func (r *identity) reconcile(ctx context.Context, cachedResource *cachev1alpha1.
6465
)
6566
}
6667
if errors.IsNotFound(err) {
67-
if err := r.createIdentitySecret(ctx, clusterName.Path(), cachedResource.Name); err != nil {
68+
if err := r.createIdentitySecret(ctx, clusterName.Path(), r.secretNamespace, cachedResource.Name); err != nil {
6869
conditions.MarkFalse(
6970
cachedResource,
7071
cachev1alpha1.CachedResourceIdentityValid,
@@ -90,7 +91,7 @@ func (r *identity) reconcile(ctx context.Context, cachedResource *cachev1alpha1.
9091
}
9192

9293
// Ref exists - make sure it's valid
93-
if err := r.updateOrVerifyIdentitySecretHash(ctx, clusterName, cachedResource); err != nil {
94+
if err := r.updateOrVerifyIdentitySecretHash(ctx, clusterName, cachedResource, r.secretNamespace); err != nil {
9495
conditions.MarkFalse(
9596
cachedResource,
9697
cachev1alpha1.CachedResourceIdentityValid,
@@ -103,5 +104,41 @@ func (r *identity) reconcile(ctx context.Context, cachedResource *cachev1alpha1.
103104
return reconcileStatusStop, err
104105
}
105106

107+
conditions.MarkTrue(cachedResource, cachev1alpha1.CachedResourceIdentityValid)
108+
106109
return reconcileStatusContinue, nil
107110
}
111+
112+
func (r *identity) updateOrVerifyIdentitySecretHash(ctx context.Context, clusterName logicalcluster.Name, cachedResource *cachev1alpha1.CachedResource, defaultSecretNamespace string) error {
113+
secret, err := r.getSecret(ctx, clusterName, cachedResource.Spec.Identity.SecretRef.Namespace, cachedResource.Spec.Identity.SecretRef.Name)
114+
if err != nil {
115+
return err
116+
}
117+
118+
hash, err := IdentityHash(secret)
119+
if err != nil {
120+
return err
121+
}
122+
123+
if cachedResource.Status.IdentityHash == "" {
124+
cachedResource.Status.IdentityHash = hash
125+
}
126+
127+
if cachedResource.Status.IdentityHash != hash {
128+
return fmt.Errorf("hash mismatch: identity secret hash %q must match status.identityHash %q", hash, cachedResource.Status.IdentityHash)
129+
}
130+
131+
return nil
132+
}
133+
134+
// TODO: This is copy from apiexport controller. We should move it to a shared location.
135+
func IdentityHash(secret *corev1.Secret) (string, error) {
136+
key := secret.Data[apisv1alpha1.SecretKeyAPIExportIdentity]
137+
if len(key) == 0 {
138+
return "", fmt.Errorf("secret is missing data.%s", apisv1alpha1.SecretKeyAPIExportIdentity)
139+
}
140+
141+
hashBytes := sha256.Sum256(key)
142+
hash := fmt.Sprintf("%x", hashBytes)
143+
return hash, nil
144+
}

0 commit comments

Comments
 (0)