You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/06/02 17:33:33 UTC
[GitHub] [camel-k] ammachado opened a new pull request, #3330: Adding basic support for `PodSecurityContext`.
ammachado opened a new pull request, #3330:
URL: https://github.com/apache/camel-k/pull/3330
<!-- Description -->
Camel-K is ignoring the securityContext part of my pod template:
```yaml
securityContext:
supplementalGroups:
- 666
```
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat: added support to podtemplate.spec.securityContext
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889551836
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
If you see the previous lines on that file, it is composing a Deployment by hand, not using one coming from an integration. All the tests on that file use a `corev1.PodTemplateSpec` spec, not the one `PodTemplateSpec` from `integration_types.go` (the one I changed).
I was debugging here on an Openshift instance, and as far as I can tell, on the POST client call where the value is sent to the Kubernetes API the value is present, but not on the return from the API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r888590837
##########
e2e/common/traits/pod_test.go:
##########
@@ -58,6 +58,30 @@ func TestPodTrait(t *testing.T) {
})
}
+func TestPodTraitWithSupplementalGroups(t *testing.T) {
Review Comment:
This pull req is great, but let's not add new `TestPod...` test function to add a test scenario to the pod trait testing. We prefer to add `t.Run("xxx", ...)` to add it to the existing `TestPodTrait()` to avoid create and destroy namespaces frequently for the e2e testing.
See https://github.com/apache/camel-k/issues/3298
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889454414
##########
e2e/common/traits/pod_test.go:
##########
@@ -34,27 +34,64 @@ import (
)
func TestPodTrait(t *testing.T) {
+
+ tc := []struct {
+ name string
+ templateName string
+ assertions func(t *testing.T, ns string, name string)
+ }{
+ {
+ name: "pod trait with env vars and volume mounts",
Review Comment:
the names should be unique based on the intents of each test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889649682
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
> > The code you've mentioned on pkg/trait/pod.go#L65 uses the PodTemplate that is resolved on https://github.com/apache/camel-k/blob/main/pkg/cmd/run.go#L583. That code wasn't changed, it was not needed.
>
> Yeah, and the behaviour from `kamel run` is mimicked here https://github.com/apache/camel-k/blob/main/pkg/trait/pod_test.go#L95-L98 so you can expect the same behaviour from the test code. That's why you wouldn't need an explicit addition of `SecurityContext` to deployment here.
Figured out what you said. I'm sorry for the misunderstanding. What's happening is that the `yaml.Unmarshal` on line 109 is initializing the field `SecurityContext` on the `podSpec`, but is not initializing the nested `SupplementalGroups`. And to be honest, I don't know why.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889650400
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
Removing lines 153-155 and replacing the import on line 25 from `"gopkg.in/yaml.v2"` to `"k8s.io/apimachinery/pkg/util/yaml"` solves the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#issuecomment-1146515029
and the test is still failing.
```
pod_test.go:65:
Expected
<[]int64 | len:0, cap:0>: nil
not to be nil
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889651388
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
cool. we finally reach the right solution ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889637354
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
The code you've mentioned in `pkg/trait/pod_test.go` is the "expected" deployment after applying the `Integration.Spec.PodTemplate.Spec`.
The code you've mentioned on `pkg/trait/pod.go#L65` uses the PodTemplate that is resolved on https://github.com/apache/camel-k/blob/main/pkg/cmd/run.go#L583. That code wasn't changed, it was not needed.
Just adding the `SecurityContext` field to the `PodSpec` template on `integration_types.go` made it work. No other changes were needed.
Then a few test cases were added, and the CRDs and deepcopy code regenerated. The rest wasn't changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889649928
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
No, the returned deployment is the "actual" case. Otherwise, these assertions you added do not test anything, as the object and value here are provided by the tester.
```
// Check if securityContext was added
assert.NotNil(t, templateSpec.Spec)
assert.NotNil(t, templateSpec.Spec.SecurityContext)
assert.NotNil(t, templateSpec.Spec.SecurityContext.SupplementalGroups)
assert.Contains(t, templateSpec.Spec.SecurityContext.SupplementalGroups, int64(666))
```
You don't want to remove the SecurityContext lines (starting from line 153) because otherwise the test doesn't pass. But as I said it should really pass without the lines.
Why it doens't pass is actually that the test `pod_test.go` itself has a potential problem and needs to be fixed. Look at the code:
https://github.com/apache/camel-k/blob/main/pkg/trait/pod_test.go#L95-L98
```
var podSpec v1.PodSpec
if podSpecTemplate != "" {
_ = yaml.Unmarshal([]byte(podSpecTemplate), &podSpec)
}
```
It actually doesn't do any unmarshalling on `podSpec` (check by inserting some println on `podSpec`). It is the root cause of the issue. We need to see why it doesn't as expected and fix it before merging the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889639230
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
Btw, you can ignore the test failure in the `openshift` workflow, which is just a known flaky test.
Now your code is ok, except only one thing that I still think the way `pod_test.go` is set up is not correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889634178
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
@ammachado But look at `(*podTrait).Appy()`:
https://github.com/apache/camel-k/blob/main/pkg/trait/pod.go#L65
It reads the `PodTemplateSpec` from `Integration` and does the job of applying the template changes to the `Deployment`. If it's hardcoded in the `Deployment` in the test settings, this step is not tested here.
Am I still missing something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi merged pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi merged PR #3330:
URL: https://github.com/apache/camel-k/pull/3330
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889508306
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
probably this part is not right. if you've implemented this feature right, PodTrait should update the deployment and you don't need to manually define them here. why do we need this part?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#issuecomment-1146445545
The file `docs/modules/ROOT/attachments/schema/integration-schema.json` is generated? If it is, how do I generate id based on the changed I've made to the CRDs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#issuecomment-1148321481
@ammachado yes please. You can cherry pick those commits and create a PR on `release-1.9.x`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889619650
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
Figured out. The problem was with an outdated `resources.go` file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889640239
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
> > The code you've mentioned in pkg/trait/pod_test.go is the "expected" deployment after applying the Integration.Spec.PodTemplate.Spec.
>
> Hmm, I don't think so... Looking at the code `pod_test.go`, the returned template from `testPodTemplateSpec()` function appears in the "actual" side of assertions in all of the tests. Where is it that the deployment returned from `createPodTest()` function is used as the "expected" value in an assertion?
Haven't noticed that before, but it is the expected case. The first time I've looked at that code I thought that the `Spec` on line 124 should come from the `podSpecTemplate` on line 108. I changed it back then, but some other tests were failing, because they're expecting the container "integration" to be available (it is default container name for the Camel-K runtime), but it is created later during the `kamel run` process, where it applies the `container` trait.
This code is just testing the parsing of pod template strings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#issuecomment-1146542781
I'm on a M1 Mac machine, I can't get past image building here. Looking for a solution...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889637520
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
A change I'd like to make is to make all fields of the `PodSpec` optional. Today the field `Containers` is not optional (the only one). I have to declare an empty `containers: []` on my pod template string in order to get it accepted on `kamel run`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889640239
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
> > The code you've mentioned in pkg/trait/pod_test.go is the "expected" deployment after applying the Integration.Spec.PodTemplate.Spec.
>
> Hmm, I don't think so... Looking at the code `pod_test.go`, the returned template from `testPodTemplateSpec()` function appears in the "actual" side of assertions in all of the tests. Where is it that the deployment returned from `createPodTest()` function is used as the "expected" value in an assertion?
Haven't noticed that before, but it is the expected case. The first time I've looked at that code I thought that the `Spec` on line 124 should come from the `podSpecTemplate` on line 24. I changed it back then, but some other tests were failing, because they're expecting the container "integration" to be available (it is default container name for the Camel-K runtime), but it is created later during the `kamel run` process, where it applies the `container` trait.
This code is just testing the parsing of pod template strings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889637520
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
A change I'd like to make is to make all fields of the `PodSpec` optional. Today the field `Containers` is not optional (the only one). I have to declare an empty `containers: []` on my pod template string to get accepted on `kamel run`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889634961
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
It was missing my changes because I had an outdated `pkg/resources/resources.go`. My tests are passing, but the build is still broken.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r888590837
##########
e2e/common/traits/pod_test.go:
##########
@@ -58,6 +58,30 @@ func TestPodTrait(t *testing.T) {
})
}
+func TestPodTraitWithSupplementalGroups(t *testing.T) {
Review Comment:
This pull req is great, but let's not add new `TestPod...` test function to add a test scenario to the pod trait testing. We prefer to add `t.Run("xxx", ...)` to add it to the existing `TestPodTrait()` to avoid create and destroy namespaces frequently for the e2e testing.
https://github.com/apache/camel-k/issues/3298
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889638843
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
> The code you've mentioned in pkg/trait/pod_test.go is the "expected" deployment after applying the Integration.Spec.PodTemplate.Spec.
Hmm, I don't think so... Looking at the code `pod_test.go`, the returned template from `testPodTemplateSpec()` function appears in the "actual" side of assertions in all of the tests. Where is it that the deployment returned from `createPodTest()` function is used as the "expected" value in an assertion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#issuecomment-1147455620
This can be backported to 1.9?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889639801
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
> The code you've mentioned on pkg/trait/pod.go#L65 uses the PodTemplate that is resolved on https://github.com/apache/camel-k/blob/main/pkg/cmd/run.go#L583. That code wasn't changed, it was not needed.
Yeah, and the behaviour from `kamel run` is mimicked here
https://github.com/apache/camel-k/blob/main/pkg/trait/pod_test.go#L95-L98
so you can expect the same behaviour from the test code. That's why you wouldn't need an explicit addition of `SecurityContext` to deployment here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] ammachado commented on a diff in pull request #3330: Adding basic support for `PodSecurityContext`.
Posted by GitBox <gi...@apache.org>.
ammachado commented on code in PR #3330:
URL: https://github.com/apache/camel-k/pull/3330#discussion_r889637354
##########
pkg/trait/pod_test.go:
##########
@@ -138,6 +150,9 @@ func createPodTest(podSpecTemplate string) (*podTrait, *Environment, *appsv1.Dep
},
},
},
+ SecurityContext: &corev1.PodSecurityContext{
Review Comment:
The code you've mentioned in `pkg/trait/pod_test.go` is the "expected" deployment after applying the `Integration.Spec.PodTemplate.Spec`.
The code you've mentioned on `pkg/trait/pod.go#L65` uses the PodTemplate that is resolved on https://github.com/apache/camel-k/blob/main/pkg/cmd/run.go#L583. That code wasn't changed, it was not needed.
Just adding the `SecurityContext` field to the `PodSpec` template on `integration_types.go` made it work. No other changes were needed.
Then a few test cases were added, and the CRDs and deepcopy code regenerated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org