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

genpolicy: add flag for cache path #593

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Conversation

3u13r
Copy link
Member

@3u13r 3u13r commented Jun 17, 2024

Upstream PR: kata-containers/kata-containers#9864
For now, this should have no change to the user.
The question is, how we now want to use this option. Also have a similar flag in our CLI or just have a general home/.config path.

TODO:

  • Wait for first upstream feedback
  • Use git format-patch to create initial patch file

@3u13r 3u13r added the no changelog PRs not listed in the release notes label Jun 17, 2024
@3u13r 3u13r force-pushed the feat/genpolicy/cache-path branch 2 times, most recently from b8b4bfd to a7cd4d0 Compare June 25, 2024 16:59
@3u13r 3u13r changed the title wip: genpolicy: add flag for cache path genpolicy: add flag for cache path Jun 25, 2024
@3u13r 3u13r marked this pull request as ready for review June 25, 2024 16:59
@3u13r 3u13r requested a review from katexochen as a code owner June 25, 2024 16:59
@@ -391,7 +391,7 @@ func addWorkloadOwnerKeyToManifest(manifst *manifest.Manifest, keyPath string) e
func generatePolicyForFile(ctx context.Context, genpolicyPath, regoPath, policyPath, yamlPath string, logger *slog.Logger) ([32]byte, error) {
args := []string{
"--raw-out",
"--use-cached-files",
"--layers-cache-file-path=./layers-cache.json",
Copy link
Member

Choose a reason for hiding this comment

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

this should be made configurable through a flag, defaulting to the file being at a default location within the configured workspaceDir.

@katexochen katexochen self-assigned this Jun 26, 2024
@3u13r 3u13r force-pushed the feat/genpolicy/cache-path branch 2 times, most recently from bae60df to 228a55c Compare June 27, 2024 21:20
@3u13r
Copy link
Member Author

3u13r commented Jun 27, 2024

Done. Do we already have a use for this e.g. somewhere in the CI? We currently don't list explicit CLI features in the docs, so there's also nothing to add here, I assume.

@3u13r 3u13r requested a review from katexochen June 28, 2024 11:40
Copy link
Member

@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

Do we already have a use for this e.g. somewhere in the CI?

As you correctly merge cache-path with workspace-dir, this change will already have effect on the dev workflow where the workspace is set. We will now generate on every just generate invocation, where we would previously often use the cache. A shift towards more correctness at the cost of performance, let's see how it goes.

cli/cmd/common.go Outdated Show resolved Hide resolved
cli/cmd/generate.go Outdated Show resolved Hide resolved
@3u13r 3u13r force-pushed the feat/genpolicy/cache-path branch from 228a55c to 0e7f658 Compare July 1, 2024 20:22
@3u13r 3u13r merged commit 9e0ffe2 into main Jul 1, 2024
8 checks passed
@3u13r 3u13r deleted the feat/genpolicy/cache-path branch July 1, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants