feat: remove shared repo volume between repo-server and cmp-server

[leoluz]

Signed-off-by: Leonardo Luz Almeida leonardo_almeida@intuit.com

Description

This PR removes the shared volume containing all application's repository files between the repo-server and cmp-server. In order to do so, it changes how the repo-server communicates with the cmp-server to generate manifests (GenerateManifests) and check if the repo is supported (MatchRepository) by the plugin.

Motivation

Before this PR, cmp-server was expected to have the same repo-server volume mounted containing all files from all cloned repos.

This extends all the security issues recently faced in the repo-server to the cmp-server as well.

Solution

In order to mitigate the problem the following changes are being implemented in this PR:

• Remove the shared tmp volume from cmp-server
• Applications files are now packed (tarball) and sent (gRPC stream) to the CMP server during manifest generation
• Applications files are now packed (tarball) and sent (gRPC stream) to the CMP server during MatchRepository validation
• CMP server creates a cryptic directory with the tarball contents and runs the manifest generation inside this directory
• After manifest generation is concluded cryptic directory is deleted with all contents
• Generated manifests are sent back to repo-server

Checklist

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Merging #8600 (1673621) into master (b7912ac) will increase coverage by 0.12%.
The diff coverage is 48.50%.

@@            Coverage Diff             @@
##           master    #8600      +/-   ##
==========================================
+ Coverage   42.66%   42.78%   +0.12%     
==========================================
  Files         184      186       +2     
  Lines       23130    23300     +170     
==========================================
+ Hits         9868     9970     +102     
- Misses      11859    11902      +43     
- Partials     1403     1428      +25     

Impacted Files	Coverage Δ
test/testutil.go	0.00% <0.00%> (ø)
cmpserver/plugin/plugin.go	47.48% <19.11%> (-4.45%)	⬇️
util/app/discovery/discovery.go	40.27% <20.00%> (-8.16%)	⬇️
util/io/files/util.go	33.33% <33.33%> (ø)
util/cmp/stream.go	51.04% <51.04%> (ø)
util/io/files/tar.go	55.78% <55.78%> (ø)
reposerver/repository/repository.go	59.23% <78.94%> (+1.83%)	⬆️
util/helm/client.go	45.23% <100.00%> (+0.09%)	⬆️
util/argo/argo.go	63.11% <0.00%> (-0.99%)	⬇️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7912ac...1673621. Read the comment docs.

[crenshaw-dev]

First pass done! I'll probably return with a closer look soon.

Comments mostly fall into these categories:

1. Use UUIDv4 in all temporary paths.
2. Return less detail in errors and favor logging instead.
3. Set up defer functions to close resources before short-cutting it with error checks.

[crenshaw-dev]

I haven't looked, but I think the CMP docs will need to be updated to remove any references to a shared volume.

[leoluz]

I haven't looked, but I think the CMP docs will need to be updated to remove any references to a shared volume.

@crenshaw-dev I updated the docs/user-guide/config-management-plugins.md file in this PR to remove the reference to the shared volume

[crenshaw-dev]

Partial second pass. I still need to go over the tar code and its tests again.

I think my biggest concern right now is sanitization of sensitive paths.

Instead of duplicating the repo-server sanitization interceptor in cmp-server, we could just rely on the repo-server's sanitizer, since all cmp-server traffic goes through repo-server. But we'd have to 1) define some unique root path in cmp-server to store all sensitive paths and 2) modify the repo-server sanitizer config to look for that new path.

[crenshaw-dev]

The panic -> delete temp dirs security mechanism assumes /tmp isn't mounted as a persistent volume. A lot of CMP users will continue to mount /tmp, just because that's how it was before.

We should:

1. nest all temp directories under /tmp/_cmp-server
2. clear out that directory on startup
3. add a note to the docs that, for security, /tmp should no longer be shared with repo-server as of version 2.3.X - this is necessary because, even if we're clearing the tmp directories on startup, it's better to avoid exposing repo-server's persistent git repos to the CMP container

[alexmt]

Awesome PR @leoluz . Add one nice to have comment.

LGTM after resolving @crenshaw-dev 's comments.

[crenshaw-dev]

LGTM!