feat: validate envFrom secret/cm references exist#63
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @Cali0707. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
/cc @aliok @jaideepr97 @matzew |
k8s-ci-robot
added
the
size/L
|
@Cali0707: GitHub didn't allow me to request PR reviews from the following users: aliok, jaideepr97. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
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-sigs/prow repository. |
jaideepr97
left a comment
jaideepr97
left a comment
There was a problem hiding this comment.
thanks @Cali0707!
one minor nit but LGTM overall
| } | ||
| if len(mcpServer.Spec.Config.EnvFrom) > 0 { | ||
| container.EnvFrom = mcpServer.Spec.Config.EnvFrom | ||
|
|
There was a problem hiding this comment.
nit: maybe the verification logic should come before the assignment of container.EnvFrom
|
/approve |
|
@Cali0707 needs a rebase |
k8s-ci-robot
added
the
needs-rebase
Cali0707
force-pushed
the
validate-cm-secret-envfrom
branch
from
8e9fb5b to
801bb17
Compare
k8s-ci-robot
removed
the
needs-rebase
|
@matzew rebased and fixed the nit from @jaideepr97 |
| EnvFrom: []corev1.EnvFromSource{ | ||
| { | ||
| ConfigMapRef: &corev1.ConfigMapEnvSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{Name: "nonexistent-configmap"}, |
There was a problem hiding this comment.
Could we have equivalent test with a SecretRef pointing to a nonexistent Secret ?
Signed-off-by: Calum Murray <cmurray@redhat.com>
Cali0707
force-pushed
the
validate-cm-secret-envfrom
branch
from
801bb17 to
d4f12e7
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aliok, Cali0707, jaideepr97, matzew The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
5 participants
Fixes #10
This finished up the validation that was mostly already implemented. In particular, while we were validating secrets/configmaps when mounting them to the deployment, we were not validating them when using in envFrom.