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

feature: get namespace name from release command #1499

Merged
merged 9 commits into from
Mar 15, 2023

Conversation

tuxerrante
Copy link
Contributor

@tuxerrante tuxerrante commented Feb 23, 2023

What type of PR is this?

/kind feature

What this PR does / why we need it:

It is an important missing setting but it also could be mandatory in enterprises environment where namespaces have naming conventions.

Which issue(s) this PR fixes:

#1498

Fixes #1498

Does this PR have test?

on ⛵ aks-test () security-profiles-operator go v1.19.5 took 5s
[15:13:52] ❯ helm install --create-namespace  --namespace tuxerrante-security security-profiles-operator deploy/helm/
WARNING: Kubernetes configuration file is group-readable. This is insecure. Location: /mnt/c/Users/alessandro.affinito/.kube/config
WARNING: Kubernetes configuration file is world-readable. This is insecure. Location: /mnt/c/Users/alessandro.affinito/.kube/config
W0223 15:14:16.478807    2097 warnings.go:70] spec.template.metadata.annotations[seccomp.security.alpha.kubernetes.io/pod]: deprecated since v1.19, non-functional in a future release; use the "seccompProfile" field instead
NAME: security-profiles-operator
LAST DEPLOYED: Thu Feb 23 15:14:11 2023
NAMESPACE: tuxerrante-security
STATUS: deployed
REVISION: 1
TEST SUITE: None

[15:24:01] ❯ kubectl get pods -n tuxerrante-security
NAME                                                  READY   STATUS    RESTARTS   AGE
security-profiles-operator-844954996d-9jnpk           1/1     Running   0          9m48s
security-profiles-operator-844954996d-gbrhh           1/1     Running   0          9m48s
security-profiles-operator-844954996d-rjbkj           1/1     Running   0          9m48s
security-profiles-operator-webhook-6b9c98b975-m2ccq   1/1     Running   0          2m45s
security-profiles-operator-webhook-6b9c98b975-szlts   1/1     Running   0          16m
security-profiles-operator-webhook-6b9c98b975-zr5ql   1/1     Running   0          16m
spod-5hszj                                            2/2     Running   0          9m29s
spod-qz74b                                            2/2     Running   0          9m29s


[15:24:59] ➜  kubectl logs -n tuxerrante-security deployments/security-profiles-operator
Found 3 pods, using pod/security-profiles-operator-844954996d-9jnpk
I0223 14:14:17.969898       1 main.go:258]  "msg"="Set logging verbosity to 0"
I0223 14:14:17.969940       1 main.go:264]  "msg"="Profiling support enabled: false"
I0223 14:14:17.969996       1 main.go:284] setup "msg"="starting component: security-profiles-operator" "buildDate"="1980-01-01T00:00:00Z" "buildTags"="netgo,osusergo,seccomp,apparmor" "cgoldFlags"="unknown" "compiler"="gc" "dependencies"="github.com/acobaugh/osrelease v0.1.0 ,github.com/aquasecurity/libbpfgo v0.4.5-libbpf-1.0.1 ,github.com/beorn7/perks v1.0.1 ,github.com/blang/semver/v4 v4.0.0 ,github.com/cert-manager/cert-manager v1.10.1 ,github.com/cespare/xxhash/v2 v2.2.0 ,github.com/containers/common v0.51.0 ,github.com/cpuguy83/go-md2man/v2 v2.0.2 ,github.com/davecgh/go-spew v1.1.1 ,github.com/emicklei/go-restful/v3 v3.8.0 ,github.com/evanphx/json-patch/v5 v5.6.0 ,github.com/fsnotify/fsnotify v1.6.0 ,github.com/go-logr/logr v1.2.3 ,github.com/go-openapi/jsonpointer v0.19.5 ,github.com/go-openapi/jsonreference v0.20.0 ,github.com/go-openapi/swag v0.22.3 ,github.com/gogo/protobuf v1.3.2 ,github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da ,github.com/golang/protobuf v1.5.2 ,github.com/google/gnostic v0.6.9 ,github.com/google/go-cmp v0.5.9 ,github.com/google/gofuzz v1.2.0 ,github.com/google/uuid v1.3.0 ,github.com/imdario/mergo v0.3.13 ,github.com/jellydator/ttlcache/v3 v3.0.1 ,github.com/josharian/intern v1.0.0 ,github.com/json-iterator/go v1.1.12 ,github.com/mailru/easyjson v0.7.7 ,github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 ,github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd ,github.com/modern-go/reflect2 v1.0.2 ,github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 ,github.com/nxadm/tail v1.4.8 ,github.com/opencontainers/runtime-spec v1.0.3-0.20220825212826-86290f6a00fb ,github.com/openshift/api v0.0.0-20221205111557-f2fbb1d1cd5e ,github.com/pjbgf/go-apparmor v0.1.1 ,github.com/pkg/errors v0.9.1 ,github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.62.0 ,github.com/prometheus/client_golang v1.14.0 ,github.com/prometheus/client_model v0.3.0 ,github.com/prometheus/common v0.37.0 ,github.com/prometheus/procfs v0.8.0 ,github.com/russross/blackfriday/v2 v2.1.0 ,github.com/seccomp/libseccomp-golang v0.10.0 ,github.com/sirupsen/logrus v1.9.0 ,github.com/spf13/pflag v1.0.5 ,github.com/urfave/cli/v2 v2.24.4 ,github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 ,golang.org/x/mod v0.8.0 ,golang.org/x/net v0.7.0 ,golang.org/x/oauth2 v0.4.0 ,golang.org/x/sync v0.1.0 ,golang.org/x/sys v0.5.0 ,golang.org/x/term v0.5.0 ,golang.org/x/text v0.7.0 ,golang.org/x/time v0.0.0-20220609170525-579cf78fd858 ,gomodules.xyz/jsonpatch/v2 v2.2.0 ,google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f ,google.golang.org/grpc v1.53.0 ,google.golang.org/protobuf v1.28.1 ,gopkg.in/inf.v0 v0.9.1 ,gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 ,gopkg.in/yaml.v2 v2.4.0 ,gopkg.in/yaml.v3 v3.0.1 ,k8s.io/api v0.25.4 ,k8s.io/apiextensions-apiserver v0.25.4 ,k8s.io/apimachinery v0.25.5 ,k8s.io/client-go v0.25.4 ,k8s.io/component-base v0.25.4 ,k8s.io/klog/v2 v2.90.0 ,k8s.io/kube-openapi v0.0.0-20220803164354-a70c9af30aea ,k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2 ,sigs.k8s.io/controller-runtime v0.13.1 ,sigs.k8s.io/gateway-api v0.5.0 ,sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 ,sigs.k8s.io/release-utils v0.7.3 ,sigs.k8s.io/structured-merge-diff/v4 v4.2.3 ,sigs.k8s.io/yaml v1.3.0 " "gitCommit"="unknown" "gitCommitDate"="unknown" "gitTreeState"="clean" "goVersion"="go1.20.1" "ldFlags"="unknown" "libbpf"="v1.1" "libseccomp"="2.5.4" "platform"="linux/amd64" "version"="0.6.1-dev"
...
I0223 14:24:59.549265       1 spod_controller.go:271] spod-config "msg"="Adding 'Updating' status to the SPOD Instance" "namespace"="tuxerrante-security" "profile"="spod"
I0223 14:25:27.507891       1 ca.go:62] spod-config "msg"="Using cert-manager as certificate provider"
I0223 14:25:27.508218       1 spod_controller.go:391] spod-config "msg"="Updating cert manager resources"
I0223 14:25:27.578325       1 spod_controller.go:398] spod-config "msg"="Updating operator webhook"
I0223 14:25:27.616648       1 spod_controller.go:404] spod-config "msg"="Updating operator daemonset"
I0223 14:25:27.640967       1 spod_controller.go:409] spod-config "msg"="Updating operator default profiles"
I0223 14:25:27.640989       1 spod_controller.go:445] spod-config "msg"="Updating metrics service"
I0223 14:25:27.651314       1 spod_controller.go:450] spod-config "msg"="Updating operator service monitor"
I0223 14:25:28.701598       1 request.go:682] Waited for 1.041750808s due to client-side throttling, not priority and fairness, request: GET:https://10.0.0.1:443/apis/acme.cert-manager.io/v1?timeout=32s
I0223 14:25:29.209567       1 spod_controller.go:455] spod-config "msg"="Service monitor resource does not seem to exist, ignoring"
I0223 14:25:29.209601       1 spod_controller.go:271] spod-config "msg"="Adding 'Updating' status to the SPOD Instance" "namespace"="tuxerrante-security" "profile"="spod"

Special notes for your reviewer:

Does this PR introduce a user-facing change?

CLI change: --namespace flag should be specified when installing the helm chart

It will install the app in the default namespace if no --namespace argument is given. Which is also the expected behaviour.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 23, 2023
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 23, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: tuxerrante / name: Affinito Alessandro (c6c0fb0)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Feb 23, 2023
@k8s-ci-robot
Copy link
Contributor

Welcome @tuxerrante!

It looks like this is your first PR to kubernetes-sigs/security-profiles-operator 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/security-profiles-operator has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 23, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @tuxerrante. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 23, 2023
@saschagrunert
Copy link
Member

/ok-to-test
/check-cla

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 23, 2023
@pjbgf
Copy link
Member

pjbgf commented Feb 24, 2023

/check-cla

@k8s-ci-robot k8s-ci-robot added do-not-merge/contains-merge-commits size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2023

Codecov Report

Merging #1499 (63e2cb5) into main (6e0590e) will increase coverage by 0.77%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1499      +/-   ##
==========================================
+ Coverage   45.03%   45.81%   +0.77%     
==========================================
  Files          61       61              
  Lines        6166     6166              
==========================================
+ Hits         2777     2825      +48     
+ Misses       3250     3215      -35     
+ Partials      139      126      -13     

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 28, 2023
@k8s-ci-robot k8s-ci-robot removed the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Feb 28, 2023
@tuxerrante tuxerrante marked this pull request as ready for review March 13, 2023 18:31
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 13, 2023
@JAORMX
Copy link
Contributor

JAORMX commented Mar 13, 2023

/retest

2 similar comments
@JAORMX
Copy link
Contributor

JAORMX commented Mar 13, 2023

/retest

@pjbgf
Copy link
Member

pjbgf commented Mar 13, 2023

/retest

Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

Just a few other nits.

Apart from that I am not sure about the namespaces removal on deploy/base/role_binding/yaml.

deploy/overlays/helm/kustomization.yaml Outdated Show resolved Hide resolved
deploy/overlays/helm/kustomization.yaml Outdated Show resolved Hide resolved
installation-usage.md Outdated Show resolved Hide resolved
installation-usage.md Outdated Show resolved Hide resolved
@pjbgf
Copy link
Member

pjbgf commented Mar 13, 2023

Apart from the tiny nits, it looks good. Just adding hold for other folks to also review.

/lgtm
/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 13, 2023
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2023
tuxerrante and others added 3 commits March 14, 2023 08:44
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Signed-off-by: Alessandro Affinito <affinito.ale@gmail.com>
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 14, 2023
@saschagrunert
Copy link
Member

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2023
Copy link
Contributor

@JAORMX JAORMX left a comment

Choose a reason for hiding this comment

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

/lgtm

thanks for working on this!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JAORMX, saschagrunert, tuxerrante

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [JAORMX,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 03c8b09 into kubernetes-sigs:main Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for dynamic namespace name in the Helm chart
6 participants