-
Notifications
You must be signed in to change notification settings - Fork 551
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
Default to legacy PSA settings #2906
Default to legacy PSA settings #2906
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awgreene 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:
Approvers can indicate their approval by writing |
302e3d3
to
257133a
Compare
pod-security.kubernetes.io/audit: restricted | ||
pod-security.kubernetes.io/warn: restricted |
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.
@joelanford I think we want these here as well, thoughts?
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.
+1
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.
+1 we do want these here, but a nit is to have these also be populated from the chart value like the enforce values, to make things easier during downstream. eg if we want different values in downstream, we'll simply change the chart values.
It's a hygiene nit since we can always change the hardcoded value too, but that seems like confusing paper trail potentially.
pod-security.kubernetes.io/audit: restricted | ||
pod-security.kubernetes.io/warn: restricted |
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.
@joelanford I think we want these here as well, thoughts? CC @anik120
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.
+1
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.
Same comment as above.
01106ef
to
8aa4e29
Compare
Signed-off-by: Alexander Greene <[email protected]>
86f8e26
to
245cf67
Compare
Problem: OLM recently introduced a few changes to default to running its workloads in a restricted mode. As a part of these changes, catalogSources built with earlier versions of OPM will not run as expected unless the catalogSource yaml is configured to run in a legacy version. Unfortunately, these legacy catalogs cannot be ran in restricted namespaces, which includes the `olm` namespace which is used to define global catalogSources. Solution: Provide users ample time to convert to the new restricted fromat by defaulting to legacy restrictions and reclassify the `olm` namespace as a baseline privilege namespace. Signed-off-by: Alexander Greene <[email protected]>
245cf67
to
a1fed5b
Compare
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.
Just a couple of nits otherwise lgtm
pod-security.kubernetes.io/audit: restricted | ||
pod-security.kubernetes.io/warn: restricted |
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.
+1 we do want these here, but a nit is to have these also be populated from the chart value like the enforce values, to make things easier during downstream. eg if we want different values in downstream, we'll simply change the chart values.
It's a hygiene nit since we can always change the hardcoded value too, but that seems like confusing paper trail potentially.
pod-security.kubernetes.io/audit: restricted | ||
pod-security.kubernetes.io/warn: restricted |
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.
Same comment as above.
a1fed5b
to
c1e2a1d
Compare
Signed-off-by: Alexander Greene <[email protected]>
c1e2a1d
to
18937ae
Compare
/lgtm |
Problem: OLM recently introduced a few changes to default to running its workloads in a restricted mode. As a part of these changes, catalogSources built with earlier versions of OPM will not run as expected unless the catalogSource yaml is configured to run in a legacy version. Unfortunately, these legacy catalogs cannot be ran in restricted namespaces, which includes the
olm
namespace which is used to define global catalogSources.Solution: Provide users ample time to convert to the new restricted fromat by defaulting to legacy restrictions and reclassify the
olm
namespace as a baseline privilege namespace.Signed-off-by: Alexander Greene [email protected]