Skip to content

Fix static analysis CI egress policy#784

Closed
christophetd wants to merge 4 commits intomainfrom
fix/static-analysis-egress-policy
Closed

Fix static analysis CI egress policy#784
christophetd wants to merge 4 commits intomainfrom
fix/static-analysis-egress-policy

Conversation

@christophetd
Copy link
Copy Markdown
Contributor

Summary

Test plan

  • Verify the "Run Go static analysis" check passes on this PR

Minosity-VR and others added 4 commits March 12, 2026 15:30
The Harden Runner egress policy was missing dl.google.com (used by
setup-go-faster to download Go binaries) and release-assets.githubusercontent.com
(used to download the staticcheck binary), causing the job to fail with exit code 22.
@christophetd christophetd requested review from a team as code owners March 25, 2026 13:23
@christophetd christophetd deleted the fix/static-analysis-egress-policy branch March 25, 2026 13:24
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4712cd41f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

func (k *KubernetesConfigImpl) populateViperOverride(src *viper.Viper, dst *viper.Viper, techniqueID string) {
dst.SetDefault("kubernetes", src.Get("kubernetes.default"))
if techniqueConfig := src.Get("kubernetes.techniques." + techniqueID); techniqueConfig != nil {
dst.Set("kubernetes", techniqueConfig)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Deep-merge technique pod config with global defaults

populateViperOverride replaces the entire kubernetes subtree with the technique-specific block, so when a technique overrides only part of pod (for example just pod.image), GetTechniquePodConfig reads only that partial map and drops default pod settings like labels/tolerations/node selector. This breaks the documented key-by-key override behavior for Kubernetes techniques that apply pod config in Go.

Useful? React with 👍 / 👎.

Comment on lines +148 to +149
if len(overrideVars) > 0 {
err = m.StateManager.WriteTerraformVariables(overrideVars)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Always rewrite persisted Terraform variable state

WarmUp only persists Terraform variables when len(overrideVars) > 0, which leaves stale .terraform-variables on disk after a later warmup --force run with no overrides. CleanUp then loads those stale values and passes them to terraform destroy, which can use a different resource graph than the most recent apply (for example when namespace creation is conditional on config) and fail to clean up resources.

Useful? React with 👍 / 👎.

operator: "Equal"
value: "security"
effect: "NoSchedule"
nodeSelector:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use parsed node_selector key in sample config

The shipped example uses nodeSelector, but the implementation reads node_selector (mapstructure:"node_selector" and Terraform override path kubernetes.pod.node_selector). Users copying this file will not get node selector overrides applied, so pod placement constraints silently fail.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants