Skip to content

Commit ed9a9a3

Browse files
filipcirtogandrpac
andauthored
CLOUDP-334941: Refactored Experimental Connection Secret Controller (#2749)
* feat: scaffold the ConnectionSecret controller chore: create tests, cleanup old code, refactor controller * chore: clean up loggin chore: cleanups after review * feat: implement generic controller to include deployment and federation * chore: remove reconciler from endpoint struct, add explicit fields * chore: include integration tests for connection secrets with Deployment, Federation and Users * experimental feature: connection secret controller * makefile fix for experimental run * makefile nit * fix fmt * Bump SDK Version * Enable experimental unit tests * Resolving missing indexer * fix lint * Eliminate experimental cloud test * adressing pr feedback * Experimental secrer controller * linter fix * Eliminate pair abstraction + removal of user endpoint * Feedback improvements and linter fix * adding more tests * Revert legacy changes * Revert spacing * revert spacing * revert spacing * solve env bug * rename experimental controller * Interface revision * format fix * Rename path * server-side apply for conflict resolution * feedback rename * change flag to uppercase * enable experimental e2e-tests * verbose CI experimental * nit * revert CI changes * bis revert CI changes * feedback improvements * nit * rename source to target * attempt to fix ci * ci fix trial * revert CI edits * renamings * refactoring * fix linter * remove obsolate indexer * bugfix * feedback improvements * feedback improvments * addition of tests * removal of stale methods * remove stale resource * refactor: tests * tests: increase coverage * remove irrelevant code * fix: remove deprecated apply usage * switch back to patch+apply * transition to new SSA apply * switch back to patch+apply * removal of force json serialier --------- Co-authored-by: andrpac <[email protected]>
1 parent 7e8ac85 commit ed9a9a3

18 files changed

+3119
-3
lines changed

Makefile

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,11 @@ manifests: fmt ## Generate manifests e.g. CRD, RBAC etc.
252252
controller-gen $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./api/..." paths="./internal/controller/..." output:crd:artifacts:config=config/crd/bases
253253
@./scripts/split_roles_yaml.sh
254254
ifdef EXPERIMENTAL
255-
controller-gen crd paths="./internal/nextapi/v1" output:crd:artifacts:config=internal/next-crds
255+
@if [ -d internal/next-crds ] && find internal/next-crds -maxdepth 1 -name '*.yaml' | grep -q .; then \
256+
controller-gen crd paths="./internal/nextapi/v1" output:crd:artifacts:config=internal/next-crds; \
257+
else \
258+
echo "No experimental CRDs found, skipping apply."; \
259+
fi
256260
endif
257261

258262
.PHONY: lint
@@ -547,7 +551,11 @@ clear-e2e-leftovers: ## Clear the e2e test leftovers quickly
547551
install-crds: ## Install CRDs in Kubernetes
548552
kubectl apply -k config/crd
549553
ifdef EXPERIMENTAL
550-
kubectl apply -f internal/next-crds/*.yaml
554+
@if [ -d internal/next-crds ] && find internal/next-crds -maxdepth 1 -name '*.yaml' | grep -q .; then \
555+
kubectl apply -f internal/next-crds/*.yaml; \
556+
else \
557+
echo "No experimental CRDs found, skipping apply."; \
558+
fi
551559
endif
552560

553561
.PHONY: set-namespace
@@ -566,7 +574,7 @@ install-credentials: set-namespace ## Install the Atlas credentials for the Oper
566574

567575
.PHONY: prepare-run
568576
prepare-run: generate vet manifests run-kind install-crds install-credentials
569-
rm bin/manager
577+
rm -f bin/manager
570578
$(MAKE) manager VERSION=$(NEXT_VERSION)
571579

572580
.PHONY: run

api/condition.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,15 @@ func HasConditionType(typ ConditionType, source []Condition) bool {
177177
return false
178178
}
179179

180+
func HasReadyCondition(conditions []Condition) bool {
181+
for _, c := range conditions {
182+
if c.Type == ReadyType && c.Status == corev1.ConditionTrue {
183+
return true
184+
}
185+
}
186+
return false
187+
}
188+
180189
// EnsureConditionExists adds or updates the condition in the copy of a 'source' slice
181190
func EnsureConditionExists(condition Condition, source []Condition) []Condition {
182191
condition.LastTransitionTime = metav1.Now()

internal/controller/registry.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/dryrun"
4848
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/featureflags"
4949
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/pointer"
50+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/v3/controller/connectionsecret"
5051
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/version"
5152
ctrlstate "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/state"
5253
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/ratelimit"
@@ -134,6 +135,7 @@ func (r *Registry) registerControllers(c cluster.Cluster, ap atlas.Provider) {
134135

135136
if version.IsExperimental() {
136137
// Add experimental controllers here
138+
reconcilers = append(reconcilers, connectionsecret.NewConnectionSecretReconciler(c, r.defaultPredicates(), ap, r.logger, r.globalSecretRef))
137139
}
138140
r.reconcilers = reconcilers
139141
}

internal/controller/watch/predicates.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,30 @@ func DefaultPredicates[T metav1.Object]() predicate.TypedPredicate[T] {
8484
IgnoreDeletedPredicate[T](),
8585
)
8686
}
87+
88+
type ReadyFunc[T any] func(obj T) bool
89+
90+
// ReadyTransitionPredicate filters out only those objects where the previous
91+
// oldObject was not ready, but the new one is, or the object was deleted.
92+
func ReadyTransitionPredicate[T any](ready ReadyFunc[T]) predicate.Predicate {
93+
return predicate.Funcs{
94+
CreateFunc: func(e event.CreateEvent) bool { return false },
95+
GenericFunc: func(e event.GenericEvent) bool { return false },
96+
DeleteFunc: func(e event.DeleteEvent) bool { return true },
97+
UpdateFunc: func(e event.UpdateEvent) bool {
98+
if e.ObjectNew == nil || e.ObjectOld == nil {
99+
return false
100+
}
101+
102+
newObj, ok := e.ObjectNew.(T)
103+
if !ok {
104+
return false
105+
}
106+
oldObj, ok := e.ObjectOld.(T)
107+
if !ok {
108+
return false
109+
}
110+
return !ready(oldObj) && ready(newObj)
111+
},
112+
}
113+
}

internal/controller/workflow/reason.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,18 @@ const (
193193
NetworkPeeringConnectionPending ConditionReason = "NetworkPeeringConnectionPending"
194194
NetworkPeeringConnectionClosing ConditionReason = "NetworkPeeringConnectionClosing"
195195
)
196+
197+
// ConnectionSecret reasons
198+
const (
199+
ConnectionSecretInvalidUsername ConditionReason = "ConnectionSecretInvalidUsername"
200+
ConnectionSecretStaleSecretsNotCleaned ConditionReason = "ConnectionSecretStaleSecretsNotCleaned"
201+
ConnectionSecretProjectIDNotLoaded ConditionReason = "ConnectionSecretProjectIDNotLoaded"
202+
ConnectionSecretConnectionTargetsNotLoaded ConditionReason = "ConnectionSecretConnectionTargetsNotLoaded"
203+
ConnectionSecretUserExpired ConditionReason = "ConnectionSecretUserExpired"
204+
ConnectionSecretNotReady ConditionReason = "ConnectionSecretNotReady"
205+
ConnectionSecretFailedToBuildData ConditionReason = "ConnectionSecretFailedToBuildData"
206+
ConnectionSecretFailedToFillData ConditionReason = "ConnectionSecretFailedToFillData"
207+
ConnectionSecretFailedDeletion ConditionReason = "ConnectionSecretFailedDeletion"
208+
ConnectionSecretFailedToUpsertSecret ConditionReason = "ConnectionSecretFailedToUpsertSecret"
209+
ConnectionSecretFailedToSetOwnerReferences ConditionReason = "ConnectionSecretFailedToSetOwnerReferences"
210+
)
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright 2025 MongoDB Inc
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//nolint:dupl
16+
package indexer
17+
18+
import (
19+
"context"
20+
21+
"go.uber.org/zap"
22+
"sigs.k8s.io/controller-runtime/pkg/client"
23+
24+
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
25+
)
26+
27+
const (
28+
AtlasDataFederationByProjectID = "atlasdatafederation.spec.projectID"
29+
)
30+
31+
type AtlasDataFederationByProjectIDIndexer struct {
32+
ctx context.Context
33+
client client.Client
34+
logger *zap.SugaredLogger
35+
}
36+
37+
func NewAtlasDataFederationByProjectIDIndexer(ctx context.Context, c client.Client, logger *zap.Logger) *AtlasDataFederationByProjectIDIndexer {
38+
return &AtlasDataFederationByProjectIDIndexer{
39+
ctx: ctx,
40+
client: c,
41+
logger: logger.Named(AtlasDataFederationByProjectID).Sugar(),
42+
}
43+
}
44+
45+
func (*AtlasDataFederationByProjectIDIndexer) Object() client.Object {
46+
return &akov2.AtlasDataFederation{}
47+
}
48+
49+
func (*AtlasDataFederationByProjectIDIndexer) Name() string {
50+
return AtlasDataFederationByProjectID
51+
}
52+
53+
func (a *AtlasDataFederationByProjectIDIndexer) Keys(object client.Object) []string {
54+
df, ok := object.(*akov2.AtlasDataFederation)
55+
if !ok {
56+
a.logger.Errorf("expected *v1.AtlasDataFederation but got %T", object)
57+
return nil
58+
}
59+
60+
if df.Spec.Project.Name != "" {
61+
project := &akov2.AtlasProject{}
62+
if err := a.client.Get(a.ctx, *df.Spec.Project.GetObject(df.Namespace), project); err != nil {
63+
a.logger.Errorf("unable to find project to index: %s", err)
64+
return nil
65+
}
66+
if project.ID() != "" {
67+
return []string{project.ID()}
68+
}
69+
}
70+
71+
return nil
72+
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright 2025 MongoDB Inc
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package indexer
16+
17+
import (
18+
"context"
19+
"sort"
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"go.uber.org/zap"
24+
"go.uber.org/zap/zapcore"
25+
"go.uber.org/zap/zaptest/observer"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/runtime"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
29+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
30+
31+
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
32+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/common"
33+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/status"
34+
)
35+
36+
func TestAtlasDataFederationByProjectIDIndexer(t *testing.T) {
37+
tests := map[string]struct {
38+
object client.Object
39+
expectedKeys []string
40+
expectedLogs []observer.LoggedEntry
41+
}{
42+
"should return nil on wrong type": {
43+
object: &akov2.AtlasStreamInstance{},
44+
expectedLogs: []observer.LoggedEntry{
45+
{
46+
Context: []zapcore.Field{},
47+
Entry: zapcore.Entry{LoggerName: AtlasDataFederationByProjectID, Level: zap.ErrorLevel, Message: "expected *v1.AtlasDataFederation but got *v1.AtlasStreamInstance"},
48+
},
49+
},
50+
},
51+
"should return nil when there is an empty project reference": {
52+
object: &akov2.AtlasDataFederation{},
53+
expectedLogs: []observer.LoggedEntry{},
54+
},
55+
"should return nil when referenced project was not found": {
56+
object: &akov2.AtlasDataFederation{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "df",
59+
Namespace: "ns",
60+
},
61+
Spec: akov2.DataFederationSpec{
62+
Project: common.ResourceRefNamespaced{
63+
Name: "not-found-project",
64+
},
65+
},
66+
},
67+
expectedLogs: []observer.LoggedEntry{
68+
{
69+
Context: []zapcore.Field{},
70+
Entry: zapcore.Entry{LoggerName: AtlasDataFederationByProjectID, Level: zap.ErrorLevel, Message: "unable to find project to index: atlasprojects.atlas.mongodb.com \"not-found-project\" not found"},
71+
},
72+
},
73+
},
74+
"should return project ID using DataFederation namespace": {
75+
object: &akov2.AtlasDataFederation{
76+
ObjectMeta: metav1.ObjectMeta{
77+
Name: "df",
78+
Namespace: "ns",
79+
},
80+
Spec: akov2.DataFederationSpec{
81+
Project: common.ResourceRefNamespaced{
82+
Name: "internal-project",
83+
},
84+
},
85+
},
86+
expectedKeys: []string{"external-project-id"},
87+
expectedLogs: []observer.LoggedEntry{},
88+
},
89+
}
90+
91+
for name, tt := range tests {
92+
t.Run(name, func(t *testing.T) {
93+
project := &akov2.AtlasProject{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Name: "internal-project",
96+
Namespace: "ns",
97+
},
98+
Status: status.AtlasProjectStatus{
99+
ID: "external-project-id",
100+
},
101+
}
102+
103+
testScheme := runtime.NewScheme()
104+
assert.NoError(t, akov2.AddToScheme(testScheme))
105+
106+
builder := fake.NewClientBuilder().WithScheme(testScheme)
107+
k8sClient := builder.WithObjects(project).Build()
108+
core, logs := observer.New(zap.DebugLevel)
109+
110+
indexer := NewAtlasDataFederationByProjectIDIndexer(context.Background(), k8sClient, zap.New(core))
111+
keys := indexer.Keys(tt.object)
112+
sort.Strings(keys)
113+
114+
assert.Equal(t, tt.expectedKeys, keys)
115+
assert.Equal(t, tt.expectedLogs, logs.AllUntimed())
116+
})
117+
}
118+
}

internal/indexer/indexer.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ func RegisterAll(ctx context.Context, c cluster.Cluster, logger *zap.Logger) err
7474
)
7575
if version.IsExperimental() {
7676
// add experimental indexers here
77+
indexers = append(indexers,
78+
NewAtlasDataFederationByProjectIDIndexer(ctx, c.GetClient(), logger),
79+
)
7780
}
7881
return Register(ctx, c, indexers...)
7982
}

internal/operator/builder_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/stretchr/testify/assert"
2525
"github.com/stretchr/testify/require"
2626
"go.uber.org/zap/zaptest"
27+
corev1 "k8s.io/api/core/v1"
2728
"k8s.io/apimachinery/pkg/runtime"
2829
"k8s.io/client-go/rest"
2930
"k8s.io/client-go/tools/record"
@@ -161,6 +162,7 @@ func TestBuildManager(t *testing.T) {
161162
t.Run(name, func(t *testing.T) {
162163
akoScheme := runtime.NewScheme()
163164
require.NoError(t, akov2.AddToScheme(akoScheme))
165+
require.NoError(t, corev1.AddToScheme(akoScheme))
164166

165167
mgrMock := &managerMock{}
166168
builder := NewBuilder(mgrMock, akoScheme, 5*time.Minute)

internal/timeutil/timeutil.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,18 @@ func MustParseISO8601(dateTime string) time.Time {
6161
func FormatISO8601(dateTime time.Time) string {
6262
return dateTime.Format("2006-01-02T15:04:05.999Z")
6363
}
64+
65+
// IsExpired parses the given ISO8601 date string and returns whether it is before now.
66+
// Returns an error if the string cannot be parsed.
67+
func IsExpired(deleteAfterDate string) (bool, error) {
68+
if deleteAfterDate == "" {
69+
return false, nil
70+
}
71+
72+
deleteAfter, err := ParseISO8601(deleteAfterDate)
73+
if err != nil {
74+
return false, err
75+
}
76+
77+
return deleteAfter.Before(time.Now()), nil
78+
}

0 commit comments

Comments
 (0)