feat: Support for preserving dependencies charts archives (#16725) #16750
Conversation
raz-amir
force-pushed
the
git_clean_exclude
branch
from
996ea6b
to
df5a876
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #16750 +/- ##
==========================================
- Coverage 49.50% 49.32% -0.18%
==========================================
Files 271 274 +3
Lines 47793 48204 +411
==========================================
+ Hits 23658 23779 +121
- Misses 21796 22075 +279
- Partials 2339 2350 +11 ☔ View full report in Codecov by Sentry. |
raz-amir
force-pushed
the
git_clean_exclude
branch
from
df5a876
to
a25ecc6
Compare
raz-amir
changed the title
raz-amir
force-pushed
the
git_clean_exclude
branch
2 times, most recently
from
d369544
to
54ec46b
Compare
raz-amir
changed the title
raz-amir
force-pushed
the
git_clean_exclude
branch
from
54ec46b
to
5246b40
Compare
raz-amir
changed the title
raz-amir
force-pushed
the
git_clean_exclude
branch
from
5246b40
to
69156f4
Compare
raz-amir
changed the title
raz-amir
changed the title
Signed-off-by: Raz Amir <[email protected]>
raz-amir
force-pushed
the
git_clean_exclude
branch
from
69156f4
to
2c4c9de
Compare
| @@ -1364,6 +1402,12 @@ func WithCMPTarExcludedGlobs(excludedGlobs []string) GenerateManifestOpt { | |||
| } | |||
| } | |||
|
|
|||
| func WithPreserveDependenciesChartsArchives(val bool) GenerateManifestOpt { | |||
There was a problem hiding this comment.
Could you add a docstring here describing the flag?
Signed-off-by: Raz Amir <[email protected]>
raz-amir
force-pushed
the
git_clean_exclude
branch
from
33d5b01
to
2596276
Compare
| @@ -209,6 +211,7 @@ func NewCommand() *cobra.Command { | |||
| command.Flags().StringVar(&streamedManifestMaxExtractedSize, "streamed-manifest-max-extracted-size", env.StringFromEnv("ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE", "1G"), "Maximum size of streamed manifest archives when extracted") | |||
| command.Flags().StringVar(&helmManifestMaxExtractedSize, "helm-manifest-max-extracted-size", env.StringFromEnv("ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE", "1G"), "Maximum size of helm manifest archives when extracted") | |||
| command.Flags().BoolVar(&disableManifestMaxExtractedSize, "disable-helm-manifest-max-extracted-size", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE", false), "Disable maximum size of helm manifest archives when extracted") | |||
| command.Flags().BoolVar(&preserveDependenciesChartsArchives, "preserve-dependencies-charts-archives", env.ParseBoolFromEnv("ARGOCD_REPO_PRESERVE_DEPENDENCIES_CHARTS_ARCHIVES", false), "Preserve dependencies charts archives during refresh") | |||
There was a problem hiding this comment.
As a new environment variable is added, can you add the variable to repo-server deployment?
There was a problem hiding this comment.
Thank you @ishitasequeira!
I added the requested change and also added to additional manifest files that have similar functionality - let me know if I should keep or remove them and stay only with the file requested by you.
There was a problem hiding this comment.
I removed that question as I just learned that the other files are auto generated
Signed-off-by: Raz Amir <[email protected]>
raz-amir
force-pushed
the
git_clean_exclude
branch
from
b6c8407
to
6b94473
Compare
Signed-off-by: Raz Amir <[email protected]>
|
@ramir-savvy something I thought about: is this change going to allow an effective bypass of repo access restrictions? For example, if App A (via Project A) has access to Parent chart and Child chart, App A will be able to pull the private Child chart. And the App B (via Project B) which has access to only the Parent chart will automatically get access to the private Child chart, because the chart is saved and cached. Is my understanding correct? |
|
This looks like a desirable change to me, but I just wanted to confirm that is there a way to opt into pulling the latest dependency on a particular run. Say I have chart A that depends on chart B at "~> 1.0.0" I have 1.0.0 in cache, so dependency is satisfied. But a bugfix is published in 1.0.1 that I would like to make sure is included going forward. How would I accomplish that? Is that already covered under a force refresh or similar? |
|
@crenshaw-dev , I believe you are right, I overlooked the project separation. |
|
@jcogilvie , good point. I think that this one is also a limitation of this feature. Today, as far as I know, there is no way to force the repo refresh. The only way to do this is to restart the repo server. @crenshaw-dev , will be great to get your input on this one |
|
@crenshaw-dev, did you get the chance to think over the above concerns? |
This PR adds a feature to allow keeping the dependency charts
tgzfiles. These files are deleted by default as part of thegit cleanoperation that takes place during thecheckoutflow in the repo server.WIth this feature enabled, and with the default exclude pattern,
git cleanwill be executed with the exclude flagHowever, keeping the
tgzfiles isn't enough, if the dependency version has advanced, sincehelm templatedoesn't check that and it succeeds anyway, and before this feature, for performance reasons,helm dependency buildis executed only ifhelm templatefails. Therefore, before runninghelm templatewe need to verify that the dependency charts versions are satisfied, so a new call tohelm dependency listis added, and if no,helm dependency rebuildwill be executed beforehelm template- just if this exclude feature is enabled.--preserve-dependencies-charts-archivesflag added toargocd-repo-serverCLI.Closes: #16725
Checklist: