Skip to content

Commit 513c7b4

Browse files
committed
feat(runtime): add drift detection for cross-region and cross-account resources
Adds protection against attempting to manage AWS resources that exist in a different region or account than the controller is configured to use. This prevents accidental resource hijacking and provides clear error messages. - Add `regionDrifted()` and `accountDrifted()` helper functions - Check for drift before creating resource manager in Reconcile - Return terminal errors when drift is detected - Add comprehensive tests for both region and account drift scenarios
1 parent 22833ca commit 513c7b4

File tree

2 files changed

+354
-67
lines changed

2 files changed

+354
-67
lines changed

pkg/runtime/reconciler.go

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -236,29 +236,6 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
236236
// helpful message to the user.
237237
acctID, needCARMLookup := r.getOwnerAccountID(desired)
238238

239-
var roleARN ackv1alpha1.AWSResourceName
240-
if teamID := r.getTeamID(desired); teamID != "" && r.cfg.FeatureGates.IsEnabled(featuregate.TeamLevelCARM) {
241-
// The user is specifying a namespace that is annotated with a team ID.
242-
// Requeue if the corresponding roleARN is not available in the Teams configmap.
243-
// Additionally, set the account ID to the role's account ID.
244-
roleARN, err = r.getRoleARN(string(teamID), ackrtcache.ACKRoleTeamMap)
245-
if err != nil {
246-
return r.handleCacheError(ctx, err, desired)
247-
}
248-
parsedARN, err := arn.Parse(string(roleARN))
249-
if err != nil {
250-
return ctrlrt.Result{}, fmt.Errorf("parsing role ARN %q from %q configmap: %v", roleARN, ackrtcache.ACKRoleTeamMap, err)
251-
}
252-
acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID)
253-
} else if needCARMLookup {
254-
// The user is specifying a namespace that is annotated with an owner account ID.
255-
// Requeue if the corresponding roleARN is not available in the Accounts configmap.
256-
roleARN, err = r.getRoleARN(string(acctID), ackrtcache.ACKRoleAccountMap)
257-
if err != nil {
258-
return r.handleCacheError(ctx, err, desired)
259-
}
260-
}
261-
262239
region := r.getRegion(desired)
263240
endpointURL := r.getEndpointURL(desired)
264241
gvk := r.rd.GroupVersionKind()
@@ -269,7 +246,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
269246
if regionDrifted(desired, region) {
270247
msg := fmt.Sprintf(
271248
"Resource already exists in region %s, but the desired state specifies region %s. ",
272-
*desired.Identifiers().Region(), region,
249+
region, desired.MetaObject().GetAnnotations()[ackv1alpha1.AnnotationRegion],
273250
)
274251
rlog.Info(
275252
msg,
@@ -295,6 +272,30 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
295272
return ctrlrt.Result{}, ackerr.NewTerminalError(errors.New(msg))
296273
}
297274

275+
var roleARN ackv1alpha1.AWSResourceName
276+
if teamID := r.getTeamID(desired); teamID != "" && r.cfg.FeatureGates.IsEnabled(featuregate.TeamLevelCARM) {
277+
// The user is specifying a namespace that is annotated with a team ID.
278+
// Requeue if the corresponding roleARN is not available in the Teams configmap.
279+
// Additionally, set the account ID to the role's account ID.
280+
roleARN, err = r.getRoleARN(string(teamID), ackrtcache.ACKRoleTeamMap)
281+
if err != nil {
282+
return r.handleCacheError(ctx, err, desired)
283+
}
284+
parsedARN, err := arn.Parse(string(roleARN))
285+
if err != nil {
286+
return ctrlrt.Result{}, fmt.Errorf("parsing role ARN %q from %q configmap: %v", roleARN, ackrtcache.ACKRoleTeamMap, err)
287+
}
288+
acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID)
289+
} else if needCARMLookup {
290+
// The user is specifying a namespace that is annotated with an owner account ID.
291+
// Requeue if the corresponding roleARN is not available in the Accounts configmap.
292+
roleARN, err = r.getRoleARN(string(acctID), ackrtcache.ACKRoleAccountMap)
293+
if err != nil {
294+
fmt.Println("test", roleARN, err)
295+
return r.handleCacheError(ctx, err, desired)
296+
}
297+
}
298+
298299
// The config pivot to the roleARN will happen if it is not empty.
299300
// in the NewResourceManager
300301
clientConfig, err := r.sc.NewAWSConfig(ctx, region, &endpointURL, roleARN, gvk)
@@ -319,10 +320,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
319320
}
320321

321322
func regionDrifted(desired acktypes.AWSResource, targetRegion ackv1alpha1.AWSRegion) bool {
322-
if desired.Identifiers().Region() == nil {
323-
return false
324-
}
325-
return *desired.Identifiers().Region() != targetRegion
323+
return desired.MetaObject().GetAnnotations()[ackv1alpha1.AnnotationRegion] != string(targetRegion)
326324
}
327325

328326
func accountDrifted(desired acktypes.AWSResource, targetAccountID ackv1alpha1.AWSAccountID) bool {
@@ -1458,6 +1456,11 @@ func getResyncPeriod(rmf acktypes.AWSResourceManagerFactory, cfg ackcfg.Config)
14581456
return defaultResyncPeriod
14591457
}
14601458

1459+
// GetCaches returns the extra caches maintained by the ACK runtime
1460+
func (r *resourceReconciler) GetCaches() ackrtcache.Caches {
1461+
return r.cache
1462+
}
1463+
14611464
// NewReconciler returns a new reconciler object
14621465
func NewReconciler(
14631466
sc acktypes.ServiceController,

0 commit comments

Comments
 (0)