-
Notifications
You must be signed in to change notification settings - Fork 21
[6/8] cli: support insecure manifests behind opt-in #2536
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
base: main
Are you sure you want to change the base?
Changes from all commits
6c4f50d
38266fe
45dbf5d
e35e886
772ceb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,9 @@ subcommands.`, | |
| cmd.Flags().Bool("insecure-enable-debug-shell-access", false, "enable the debug shell service in the pod CVM to get access from container to guest VM") | ||
| cmd.Flags().Bool("calculate-pod-memory", false, "calculate pod memory based on image layer sizes and container resource limits") | ||
| cmd.Flags().StringP("output", "o", "", "output file for generated YAML") | ||
| if insecureRuntimesAllowed() { | ||
| cmd.Flags().Bool("INSECURE", false, "allow generation for insecure (non-CC) runtimes") | ||
| } | ||
| must(cmd.MarkFlagFilename("policy", "rego")) | ||
| must(cmd.MarkFlagFilename("settings", "json")) | ||
| must(cmd.MarkFlagFilename("manifest", "json")) | ||
|
|
@@ -144,6 +147,10 @@ func runGenerate(cmd *cobra.Command, args []string) error { | |
| usedPlatforms.Add(flags.referenceValuesPlatform) | ||
| } | ||
|
|
||
| if err := validateInsecurePlatforms(usedPlatforms, flags.allowInsecureRuntimes); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // generate a manifest by checking if a manifest exists and using that, | ||
| // or otherwise using a default. | ||
| var mnf *manifest.Manifest | ||
|
|
@@ -294,7 +301,7 @@ func runGenerate(cmd *cobra.Command, args []string) error { | |
| return nil | ||
| } | ||
|
|
||
| // mapContrastWorkloads applies the given function to all workloads with a Contrast runtime class. | ||
| // mapContrastWorkloads applies the given function to all workloads with the 'contrast-cc' or 'contrast-insecure' runtime class. | ||
| // The callback receives an apply configuration together with the file path and index the unstructured object has in the file map. | ||
| // Changes to the apply configuration are not applied to the original unstructured object. | ||
| func mapContrastWorkloads(fileMap map[string][]*unstructured.Unstructured, f func(res any, path string, idx int) (any, error)) error { | ||
|
|
@@ -343,6 +350,16 @@ func isCoordinator(resource any) bool { | |
| return false | ||
| } | ||
|
|
||
| func patchCoordinatorAllowInsecure(resource any) { | ||
| r, ok := resource.(*applyappsv1.StatefulSetApplyConfiguration) | ||
| if !ok || !isCoordinator(resource) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return | ||
| } | ||
| if len(r.Spec.Template.Spec.Containers) > 0 { | ||
| r.Spec.Template.Spec.Containers[0].WithEnv(kuberesource.NewEnvVar(allowInsecureEnvVar, "1")) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't idempotent (i.e. running insecure generate twice appends this twice). Since all our other mutators are this should be too.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, can we really assume the coordinator is always at index |
||
| } | ||
| } | ||
|
|
||
| func runVerifiers(fileMap map[string][]*unstructured.Unstructured, verifiers []verifier.Verifier) error { | ||
| var findings error | ||
| for _, v := range verifiers { | ||
|
|
@@ -428,7 +445,7 @@ func extractTargets(paths []string, configFile io.Writer, logger *slog.Logger) ( | |
| } | ||
| } | ||
| if len(fileMap) == 0 { | ||
| return nil, "", fmt.Errorf("no .yml/.yaml files with 'contrast-cc' runtime found") | ||
| return nil, "", fmt.Errorf("no .yml/.yaml files with 'contrast-cc' or 'contrast-insecure' runtime found") | ||
| } | ||
|
|
||
| extraData, err := kuberesource.EncodeUnstructured(extraResources) | ||
|
|
@@ -586,6 +603,9 @@ func patchTargets(fileMap map[string][]*unstructured.Unstructured, imageReplacem | |
| if flags.injectImageStore { | ||
| kuberesource.AddImageStore([]any{res}) | ||
| } | ||
| if flags.allowInsecureRuntimes { | ||
| patchCoordinatorAllowInsecure(res) | ||
| } | ||
|
Comment on lines
+606
to
+608
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may be overly cautious, but the flag alone should not be enough in an all-secure deployment. Maybe the if should additionally check that the usedPlatforms are all insecure. |
||
|
|
||
| kuberesource.PatchImages([]any{res}, replacements) | ||
|
|
||
|
|
@@ -631,6 +651,16 @@ func injectServiceMesh(resource any, memoryProfile kuberesource.MemoryProfile) e | |
| return nil | ||
| } | ||
|
|
||
| func validateInsecurePlatforms(usedPlatforms kuberesource.PlatformCollection, allowInsecure bool) error { | ||
| if !slices.ContainsFunc(usedPlatforms.Platforms(), platforms.IsInsecure) { | ||
| return nil | ||
| } | ||
| if !allowInsecure { | ||
| return fmt.Errorf("insecure runtime platforms detected but --INSECURE flag not set (the flag is only available with the %s environment variable set to true)", allowInsecureEnvVar) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateOutputFile(outputFile string) error { | ||
| if outputFile == "" { | ||
| return nil | ||
|
|
@@ -758,7 +788,17 @@ func patchRuntimeClassName(defaultRuntimeHandler string) func(*applycorev1.PodSp | |
| if spec == nil || spec.RuntimeClassName == nil { | ||
| return spec, nil | ||
| } | ||
| if *spec.RuntimeClassName == "kata-cc-isolation" || *spec.RuntimeClassName == "contrast-cc" { | ||
| if *spec.RuntimeClassName == "kata-cc-isolation" || *spec.RuntimeClassName == "contrast-cc" || *spec.RuntimeClassName == "contrast-insecure" { | ||
| // Only allow the bare runtime class names if the default runtime handler is compatible. | ||
| // For example, `contrast-cc` should only resolve when `--reference-values` is set to a CC-enabled platform, | ||
| // and `contrast-insecure` should only resolve when `--reference-values` is set to an insecure platform. | ||
| if *spec.RuntimeClassName == "contrast-insecure" && !strings.HasPrefix(defaultRuntimeHandler, "contrast-insecure-") { | ||
| return nil, fmt.Errorf("bare 'contrast-insecure' runtime class requires --reference-values to be set to an insecure platform") | ||
| } | ||
| if (*spec.RuntimeClassName == "contrast-cc" || *spec.RuntimeClassName == "kata-cc-isolation") && | ||
| strings.HasPrefix(defaultRuntimeHandler, "contrast-insecure-") { | ||
| return nil, fmt.Errorf("bare %q runtime class is incompatible with insecure --reference-values platform %q", *spec.RuntimeClassName, defaultRuntimeHandler) | ||
| } | ||
| spec.RuntimeClassName = &defaultRuntimeHandler | ||
| if kuberesource.PodSpecRequiresGPU(spec) { | ||
| platform, err := platforms.FromRuntimeClassString(*spec.RuntimeClassName) | ||
|
|
@@ -773,7 +813,7 @@ func patchRuntimeClassName(defaultRuntimeHandler string) func(*applycorev1.PodSp | |
| } | ||
| return spec, nil | ||
| } | ||
| if !strings.HasPrefix(*spec.RuntimeClassName, "contrast-cc-") { | ||
| if !kuberesource.IsContrastPod(spec) { | ||
| return spec, nil | ||
| } | ||
| overridePlatform, err := platforms.FromRuntimeClassString(*spec.RuntimeClassName) | ||
|
|
@@ -961,6 +1001,7 @@ type generateFlags struct { | |
| injectImageStore bool | ||
| insecureEnableDebugShell bool | ||
| calculatePodMemory bool | ||
| allowInsecureRuntimes bool | ||
| outputFile string | ||
| } | ||
|
|
||
|
|
@@ -1066,6 +1107,13 @@ func parseGenerateFlags(cmd *cobra.Command) (*generateFlags, error) { | |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| allowInsecureRuntimes := false | ||
| if cmd.Flags().Lookup("INSECURE") != nil { | ||
| allowInsecureRuntimes, err = cmd.Flags().GetBool("INSECURE") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| outputFile, err := cmd.Flags().GetString("output") | ||
| if err != nil { | ||
| return nil, err | ||
|
|
@@ -1093,6 +1141,7 @@ func parseGenerateFlags(cmd *cobra.Command) (*generateFlags, error) { | |
| injectImageStore: injectImageStore, | ||
| insecureEnableDebugShell: insecureEnableDebugShell, | ||
| calculatePodMemory: calculatePodMemory, | ||
| allowInsecureRuntimes: allowInsecureRuntimes, | ||
| outputFile: outputFile, | ||
| }, nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where the discussion around envvar presence vs value ultimately landed. However, given that in the coordinator we now have:
one of these sites needs to be adjusted so that the semantics are the same.