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.

Sign up for GitHub

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

Jump to bottom

ApiServerSource creates receive adapter deployments dynamically without setting SecurityContext #6787

Closed
matzew opened this issue Feb 28, 2023 · 4 comments · Fixed by #6788
Closed

ApiServerSource creates receive adapter deployments dynamically without setting SecurityContext #6787

matzew opened this issue Feb 28, 2023 · 4 comments · Fixed by #6788
Assignees
@matzew
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@matzew
Copy link
Member

matzew commented Feb 28, 2023

Describe the bug

For RA of ApiServerSource we do completelt lack the SecurityContext of the Container obj on each of the "k8s source"'s deployment:

https://github.com/knative/eventing/blob/main/pkg/reconciler/apiserversource/resources/receive_adapter.go#L87-L107

In #6533 we did address some PodSecurityPolicy enhancement, but looks like we missed the ApiServerSource
@matzew matzew added the kind/bug Categorizes issue or PR as related to a bug. label Feb 28, 2023
@matzew
Copy link
Member Author

matzew commented Feb 28, 2023

/cc @evankanderson FYI

@evankanderson
Copy link
Member

I'm not sure we have a consistent pattern for enhancing these created deployments, so it's not surprising we might miss some things.

@matzew
Copy link
Member Author

matzew commented Feb 28, 2023

/assign matzew

@matzew matzew mentioned this issue Feb 28, 2023
Merged
5 tasks
@matzew
Copy link
Member Author

matzew commented Feb 28, 2023

@evankanderson I saw that for serving you introduced a feature-flag, but the config there is generally more broad.

For our internal deployment, I am thinking to just add the bits, similar to the static manifests we updated late 2022.

knative-prow bot pushed a commit that referenced this issue Mar 3, 2023
@matzew
SC settings for ApiServerSource's Receive Adapter's container/deploym…
397387e 
…ent (#6788)

Fixes #6787

<!-- Please include the 'why' behind your changes if no issue exists -->

## Proposed Changes

<!-- Please categorize your changes:
- 🎁 Add new feature
- 🐛 Fix bug
- 🧹 Update or clean up current behavior
- 🗑️ Remove feature or internal logic
-->

- similar to our static manifests we set the required SC bits (see:
#6533), except SeccompProfile

### Pre-review Checklist

<!-- If these boxes are not checked, you will be asked to complete these
requirements or explain why they do not apply to your PR. -->

- [ ] **At least 80% unit test coverage**
- [ ] **E2E tests** for any new behavior
- [ ] **Docs PR** for any user-facing impact
- [ ] **Spec PR** for any new API feature
- [ ] **Conformance test** for any change to the spec

**Release Note**

<!--
:page_facing_up: If this change has user-visible impact, write a release
note in the block
below. Include the string "action required" if additional action is
required of
users switching to the new release, for example in case of a breaking
change.

Write as if you are speaking to users, not other Knative contributors.
If this
change has no user-visible impact, no release note is needed.
-->

```release-note
SecurityContext settings for ApiServerSource's Receive Adapter's container/deployment 
```


**Docs**

<!--
:book: If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.
-->

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
vishal-chdhry pushed a commit to vishal-chdhry/eventing that referenced this issue Mar 14, 2023
@matzew @vishal-chdhry
SC settings for ApiServerSource's Receive Adapter's container/deploym…
637dd70 
…ent (knative#6788)

Fixes knative#6787

<!-- Please include the 'why' behind your changes if no issue exists -->

## Proposed Changes

<!-- Please categorize your changes:
- 🎁 Add new feature
- 🐛 Fix bug
- 🧹 Update or clean up current behavior
- 🗑️ Remove feature or internal logic
-->

- similar to our static manifests we set the required SC bits (see:
knative#6533), except SeccompProfile

### Pre-review Checklist

<!-- If these boxes are not checked, you will be asked to complete these
requirements or explain why they do not apply to your PR. -->

- [ ] **At least 80% unit test coverage**
- [ ] **E2E tests** for any new behavior
- [ ] **Docs PR** for any user-facing impact
- [ ] **Spec PR** for any new API feature
- [ ] **Conformance test** for any change to the spec

**Release Note**

<!--
:page_facing_up: If this change has user-visible impact, write a release
note in the block
below. Include the string "action required" if additional action is
required of
users switching to the new release, for example in case of a breaking
change.

Write as if you are speaking to users, not other Knative contributors.
If this
change has no user-visible impact, no release note is needed.
-->

```release-note
SecurityContext settings for ApiServerSource's Receive Adapter's container/deployment 
```


**Docs**

<!--
:book: If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.
-->

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matzew matzew

Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SC settings for ApiServerSource's Receive Adapter's container/deployment matzew/eventing
2 participants
@matzew @evankanderson