Skip to content
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

Secret updates/deletes only trigger DAG rebuilds if referenced #4792

Merged
merged 3 commits into from
Oct 24, 2022

Conversation

skriss
Copy link
Member

@skriss skriss commented Oct 13, 2022

On Secret updates and deletes, only
trigger DAG rebuilds if the Secret
is referenced by an Ingress, HTTPProxy,
Gateway or global config.

Closes #4386.

Signed-off-by: Steve Kriss krisss@vmware.com

@skriss skriss added this to the 1.24.0 milestone Oct 13, 2022
@skriss
Copy link
Member Author

skriss commented Oct 13, 2022

This is at least partly covered by existing tests, but I'll take another look to see if coverage should be improved anywhere.

@skriss skriss added the release-note/small A small change that needs one line of explanation in the release notes. label Oct 13, 2022
@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #4792 (fa52ad0) into main (ec00824) will increase coverage by 0.04%.
The diff coverage is 91.48%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4792      +/-   ##
==========================================
+ Coverage   76.16%   76.20%   +0.04%     
==========================================
  Files         140      140              
  Lines       16914    16920       +6     
==========================================
+ Hits        12882    12894      +12     
+ Misses       3780     3776       -4     
+ Partials      252      250       -2     
Impacted Files Coverage Δ
internal/dag/dag.go 96.62% <ø> (ø)
internal/dag/cache.go 92.37% <85.36%> (+0.88%) ⬆️
internal/dag/secret.go 75.62% <95.45%> (+1.23%) ⬆️
internal/dag/extension_processor.go 95.79% <100.00%> (ø)
internal/dag/gatewayapi_processor.go 94.72% <100.00%> (ø)
internal/dag/httpproxy_processor.go 93.20% <100.00%> (ø)
internal/dag/ingress_processor.go 97.07% <100.00%> (ø)

@sunjayBhatia
Copy link
Member

sunjayBhatia commented Oct 14, 2022

This is at least partly covered by existing tests, but I'll take another look to see if coverage should be improved anywhere.

yeah i think there is a featuretest that unrelated secret additions dont cause config version changes, might be good to test removal of an unreferenced secret doesnt cause a dag build etc. for some more coverage:

// s2 is not referenced by any active ingress object.
s2 := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "unrelated",
Namespace: "default",
},
Type: "kubernetes.io/tls",
Data: s1.Data,
}
rh.OnAdd(s2)
c.Request(secretType).Equals(&envoy_discovery_v3.DiscoveryResponse{
Resources: resources(t, secret(s1)),
})
assertEqualVersion(t, "2", res)

@skriss
Copy link
Member Author

skriss commented Oct 17, 2022

@sunjayBhatia good call, added to that test and also found/fixed some bugs in that test that were resulting in it not testing what we thought it was 😅

@skriss skriss marked this pull request as ready for review October 21, 2022 16:18
@skriss skriss requested a review from a team as a code owner October 21, 2022 16:18
@skriss skriss requested review from tsaarni, stevesloka and sunjayBhatia and removed request for a team October 21, 2022 16:18
On Secret updates and deletes, only
trigger DAG rebuilds if the Secret
is referenced by an Ingress, HTTPProxy,
Gateway or global config.

Closes projectcontour#4386.

Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss force-pushed the pr-reduce-secrets-churn branch from 691d57c to fa52ad0 Compare October 21, 2022 16:38
@skriss skriss merged commit 7633c05 into projectcontour:main Oct 24, 2022
@skriss skriss deleted the pr-reduce-secrets-churn branch October 24, 2022 17:10
moeyui1 pushed a commit to moeyui1/contour that referenced this pull request Oct 26, 2022
…ctcontour#4792)

On Secret updates and deletes, only
trigger DAG rebuilds if the Secret
is referenced by an Ingress, HTTPProxy,
Gateway or global config.

Closes projectcontour#4386.

Signed-off-by: Steve Kriss <krisss@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unrelated secret changes trigger contour envoy xds communication and status update on httpproxies
2 participants