Skip to content

chore: Ensure controller understands both ENABLE & ENABLED contributorInsights #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apis/v1alpha1/ack-generate-metadata.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
ack_generate_info:
build_date: "2025-04-01T02:35:33Z"
build_hash: 980cb1e4734f673d16101cf55206b84ca639ec01
build_date: "2025-04-08T22:25:51Z"
build_hash: 0909e7f0adb8ffe4120a8c20d5d58b991f2539e9
go_version: go1.24.1
version: v0.44.0
version: v0.44.0-3-g0909e7f
api_directory_checksum: 2f94829f12ad90f9c36c48823459c161c1670093
api_version: v1alpha1
aws_sdk_go_version: v1.32.6
generator_config_info:
file_checksum: e7e79c3c7c21273967ca24947fda0f26b344c8f0
file_checksum: cc3489b53a45170d339a4de0d7d7ec0aa788955e
original_file_name: generator.yaml
last_modification:
reason: API generation
2 changes: 2 additions & 0 deletions apis/v1alpha1/generator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ resources:
from:
operation: UpdateContributorInsights
path: ContributorInsightsAction
compare:
is_ignored: true
exceptions:
errors:
404:
Expand Down
2 changes: 2 additions & 0 deletions generator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ resources:
from:
operation: UpdateContributorInsights
path: ContributorInsightsAction
compare:
is_ignored: true
exceptions:
errors:
404:
Expand Down
7 changes: 0 additions & 7 deletions pkg/resource/table/delta.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 34 additions & 11 deletions pkg/resource/table/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,16 @@ func customPreCompare(
// This will ensure controller does not act on this field
// if user is unaware of it.
if a.ko.Spec.ContributorInsights == nil {
a.ko.Spec.ContributorInsights = b.ko.Spec.ContributorInsights
return
}
// latestInsight will always be either ENABLED or DISABLED, since we requeue at readOne if its not
Copy link
Member

@rushmash91 rushmash91 Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if the desired ContributorInsights is nil, we do not compare?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

// either
desiredInsight, _ := ensureContibutorInsight(a)
latestInsight, _ := ensureContibutorInsight(b)
if desiredInsight != latestInsight {
delta.Add("Spec.ContributorInsights", a.ko.Spec.ContributorInsights, b.ko.Spec.ContributorInsights)
}

}

// equalAttributeDefinitions return whether two AttributeDefinition arrays are equal or not.
Expand Down Expand Up @@ -775,19 +783,33 @@ func (rm *resourceManager) setContributorInsights(
return err
}

switch resp.ContributorInsightsStatus {
case svcsdktypes.ContributorInsightsStatusEnabled:
ko.Spec.ContributorInsights = aws.String(string(svcsdktypes.ContributorInsightsActionEnable))
case svcsdktypes.ContributorInsightsStatusDisabled:
ko.Spec.ContributorInsights = aws.String(string(svcsdktypes.ContributorInsightsActionDisable))
default:
// Only manage ContributorInsights if it is defined
if ko.Spec.ContributorInsights != nil {
ko.Spec.ContributorInsights = aws.String(string(resp.ContributorInsightsStatus))

}

return nil
}

// ensureContibutorInsight ensures the controller understands ENABLE and ENABLED are the same.
// We choose to return ContributorInsightsAction because that is the enum used to update.
func ensureContibutorInsight(r *resource) (svcsdktypes.ContributorInsightsAction, error) {
insight := svcsdktypes.ContributorInsightsActionDisable
if r.ko.Spec.ContributorInsights != nil {
// We will allow users to provide values ENABLE, ENABLED, DISABLE, DISABLED
switch *r.ko.Spec.ContributorInsights {
case string(svcsdktypes.ContributorInsightsActionEnable), string(svcsdktypes.ContributorInsightsStatusEnabled):
insight = svcsdktypes.ContributorInsightsActionEnable
case string(svcsdktypes.ContributorInsightsActionDisable), string(svcsdktypes.ContributorInsightsStatusDisabled):
insight = svcsdktypes.ContributorInsightsActionDisable
default:
return "", fmt.Errorf("invalid ContributorInsights value: %s", *r.ko.Spec.ContributorInsights)
}
}

return insight, nil
}

func (rm *resourceManager) updateContributorInsights(
ctx context.Context,
r *resource,
Expand All @@ -797,9 +819,10 @@ func (rm *resourceManager) updateContributorInsights(
defer func() {
exit(err)
}()
insight := svcsdktypes.ContributorInsightsActionDisable
if r.ko.Spec.ContributorInsights != nil {
insight = svcsdktypes.ContributorInsightsAction(*r.ko.Spec.ContributorInsights)

insight, err := ensureContibutorInsight(r)
if err != nil {
return fmt.Errorf("failed preparing contributorInsight: %v", err)
}

_, err = rm.sdkapi.UpdateContributorInsights(
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/resources/table_insights.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ spec:
tableName: $TABLE_NAME
billingMode: PAY_PER_REQUEST
tableClass: STANDARD
contributorInsights: ENABLE
contributorInsights: ENABLED
attributeDefinitions:
- attributeName: Bill
attributeType: S
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/tests/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ def test_enable_ttl(self, table_lsi):
# Patch k8s resource
k8s.patch_custom_resource(ref, updates)

time.sleep(MODIFY_WAIT_AFTER_SECONDS)

table.wait_until(
table_name,
table.ttl_on_attribute_matches("ForumName"),
Expand Down Expand Up @@ -480,7 +482,7 @@ def test_update_insights(self, table_insights):

# Check DynamoDB Table exists
assert self.table_exists(table_name)
assert cr['spec']['contributorInsights'] == "ENABLE"
assert cr['spec']['contributorInsights'] == "ENABLED"
assert self.table_insight_status(table_name, "ENABLED")

# Set provisionedThroughput
Expand Down