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

feat(agent,rapid-response): set metadata.namespace on all namespaced items #1259

Merged

Conversation

aroberts87
Copy link
Collaborator

What this PR does / why we need it:

There were a handful of places in the charts related to sysdig-deploy where the metadata.namespace field was not being explicitly set. This is works out fine as Helm will set those fields itself during the helm install process, but does not do so during a call to helm template. This has been found to cause issues with come CI platforms that generate manifest files via helm template and then apply them later with kubectl apply -f .... What has been observed is that these select CI platforms will patch the manifests of namespaced items that do not have metadata.namespace set, and will do so with an unexpected value. When the subsequent kubectl apply -f ... command is run that supplies the desired namespace, the command fails because the namespace field is already set on these items and is different from what is being requested. This change ensures that all namespaced items have the values of metadata.namespace explicitly set to prevent the above issue.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Title of the PR starts with type and scope, (e.g. feat(agent,node-analyzer,sysdig-deploy):)
  • Chart Version bumped for the respective charts
  • Check GithubAction checks (like lint) to avoid merge-check stoppers

Check Contribution guidelines in README.md for more insight.

@aroberts87 aroberts87 requested a review from a team as a code owner July 24, 2023 19:53
@github-actions github-actions bot added the no-tests Chart templates modified without test changes label Jul 24, 2023
@aroberts87 aroberts87 requested a review from dark-vex July 24, 2023 19:53
…items

There were a handful of places in the charts related to `sysdig-deploy` where
the `metadata.namespace` field was not being explicitly set. This is works out
fine as Helm will set those fields itself during the `helm install` process,
but does not do so during a call to `helm template`. What has been discovered
is that for the workflow of generating manifest files via `helm template` and
then applying them later with `kubectl apply -f ...`, certain CI utilities will
patch the manifests not explicitly setting their namespace with a placeholder.
When the subsequent `kubectl apply -f ...` command is run that supplies the
desired namespace, the command fails because the namespace field is already set
on some constructs and is different from what is being requested. This change ensures that all namespaced items have the values of `metadata.namespace`
explicitly set to prevent the above issue.
@aroberts87 aroberts87 force-pushed the feat/smagent-5239_unify_metadata.namespace_handling branch from 991b93c to fa80c0a Compare July 24, 2023 19:57
Copy link
Collaborator

@dark-vex dark-vex left a comment

Choose a reason for hiding this comment

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

Thanks @aroberts87 LGTM

@aroberts87 aroberts87 merged commit 13dc488 into master Jul 25, 2023
5 checks passed
@aroberts87 aroberts87 deleted the feat/smagent-5239_unify_metadata.namespace_handling branch July 25, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-tests Chart templates modified without test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants