[SRVKS-1179] enable secret filter informer by default #2445
Conversation
| // TODO: Annotation is deprecated, remove in future releases | ||
| secretInformerFilteringAnnotation = "serverless.openshift.io/enable-secret-informer-filtering" | ||
| ) | ||
| // TODO: Maybe decide to fetch from net-kourier deps instead |
There was a problem hiding this comment.
I am thinking of moving that const to knative.dev/pkg upstream or something so net-istio could benefit from it. So not eager to bring in the net-kourier dep.
There was a problem hiding this comment.
Yeah, probably better to move it to pkg upstream. Do you want to create a JIRA for that. We'll probably forget the TODO here.
skonto
requested review from
ReToCode
and removed request for
jcrossley3 and
dsimansk
skonto
force-pushed
the
enable_secret_filter_informers_by_default
branch
from
a11bd47 to
dab1bc0
Compare
| @@ -41,7 +36,8 @@ func enableSecretInformerFilteringTransformers(ks base.KComponent) []mf.Transfor | |||
|
|
|||
| func injectLabelIntoInternalEncryptionSecret() mf.Transformer { | |||
| return func(u *unstructured.Unstructured) error { | |||
| if u.GetKind() == "Secret" && u.GetName() == ServingInternalCertName { | |||
| //nolint:staticcheck // ignore the deprecation until internal encryption is implemented downstream | |||
There was a problem hiding this comment.
As good as it can gets: golangci/golangci-lint#741.
|
/test ? |
|
@skonto: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In 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/test-infra repository. |
|
/test 414-mesh-e2e-aws-ocp-414 |
|
/test 414-mesh-upgrade-aws-ocp-414 |
| // TODO: Annotation is deprecated, remove in future releases | ||
| secretInformerFilteringAnnotation = "serverless.openshift.io/enable-secret-informer-filtering" | ||
| ) | ||
| // TODO: Maybe decide to fetch from net-kourier deps instead |
There was a problem hiding this comment.
Yeah, probably better to move it to pkg upstream. Do you want to create a JIRA for that. We'll probably forget the TODO here.
| if tf == nil { | ||
| t.Errorf("Secret transformer should not be nil") | ||
| } | ||
| if c.shouldAddLabelToSecret && tf == nil { |
There was a problem hiding this comment.
Hm, I'd say the old version was more readable. The else if part is a bit hard to parse.
There was a problem hiding this comment.
This part was not tested in the past. We missed that case so it is complete now. The else part means that if we should not add a label to a secret for this case we should not have generated a transformer.
Let me check if I can improve it.
skonto
force-pushed
the
enable_secret_filter_informers_by_default
branch
from
a58603c to
bf63b97
Compare
|
/hold until docs team verifies it can document it. See discussion here. |
|
/unhold Update: docs team has set this as a blocker for 1.32, see https://redhat-internal.slack.com/archives/CGUG69160/p1706703811019039?thread_ts=1706018885.016239&cid=CGUG69160. |
skonto
force-pushed
the
enable_secret_filter_informers_by_default
branch
from
bf63b97 to
6dae262
Compare
|
@ReToCode ready again, had to rebase. |
|
@skonto: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ReToCode, skonto 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 |
openshift-merge-bot
bot
merged commit 88d09c3
into
openshift-knative:main
Fixes JIRA #SRVKS-1179
Proposed Changes