argoproj / gitops-engine

Democratizing GitOps
https://pkg.go.dev/github.com/argoproj/gitops-engine?tab=subdirectories
Apache License 2.0
1.68k stars 252 forks source link

fix: cache gvk responses when checking sync task permissions #448

Open crenshaw-dev opened 2 years ago

crenshaw-dev commented 2 years ago

It's likely that syncs contain multiple tasks with the same gvk. This will make sure we only hit the k8s API once per gvk per sync operation.

codecov[bot] commented 2 years ago

Codecov Report

Merging #448 (061ac32) into master (a56a803) will decrease coverage by 0.01%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #448      +/-   ##
==========================================
- Coverage   55.27%   55.26%   -0.02%     
==========================================
  Files          41       41              
  Lines        4452     4453       +1     
==========================================
  Hits         2461     2461              
- Misses       1802     1803       +1     
  Partials      189      189              
Impacted Files Coverage Δ
pkg/sync/sync_context.go 72.11% <0.00%> (-0.09%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jessesuen commented 2 years ago

As part of this change, should we remove caching introduced here: https://github.com/argoproj/gitops-engine/pull/449/files#diff-74e77a4722803a7d3f87b956ec81b82d3c5a0dd50de299922aa2208fec57e6e9R693-R720

crenshaw-dev commented 2 years ago

@jessesuen we should. I'll do that when https://github.com/argoproj/gitops-engine/pull/452 is merged

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication