Support secret filtering informer #862

skonto · 2022-06-23T15:28:02Z

Changes

Similar to Add filtering support for secret informer in ingress controller net-istio#920
Tested via the internal encrypted traffic feature.

Fixes #

Release Note

Secret informer in net-kourier ingress controller can filter secrets based on the existence of a label key`networking.internal.knative.dev/certificate-uid`. Users are required to add the label key for custom secrets defined in ingresses.
For special cases users are recommended to reach out to Knative community.
In case of AutoTLS provided by Knative net-certmanager or net-http01 this is done transparently.
To enable this behavior at net-kourier set env var ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UI to true. 
This will be available by default in future releases.

Docs

Secret informer in net-kourier ingress controller can filter secrets based on the existence of a label key`networking.internal.knative.dev/certificate-uid"`. Users are required to add the label key for custom secrets defined in ingresses.
For special cases users are recommended to reach out to Knative community.
In case of AutoTLS provided by Knative net-certmanager or net-http01 this is done transparently.
To enable this behavior at net-kourier set env var ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UI to true. 
This will be available by default in future releases.

knative-prow · 2022-06-23T15:28:04Z

@skonto: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

In response to this:

Changes

Similar to Add filtering support for secret informer in ingress controller net-istio#920
Tested via the internal encrypted traffic feature.

/kind

Fixes #

Release Note

Secret informer in net-kourier ingress controller can filter secrets based on the existence of a label key`networking.internal.knative.dev/certificate-uid`. Users are required to add the label key for custom secrets defined in ingresses.
For special cases users are recommended to reach out to Knative community.
In case of AutoTLS provided by Knative net-certmanager or net-http01 this is done transparently.
To enable this behavior at net-istio set env var ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UI to true. 
This will be available by default in future releases.

Docs

Secret informer in net-kourier ingress controller can filter secrets based on the existence of a label key`networking.internal.knative.dev/certificate-uid"`. Users are required to add the label key for custom secrets defined in ingresses.
For special cases users are recommended to reach out to Knative community.
In case of AutoTLS provided by Knative net-certmanager or net-http01 this is done transparently.
To enable this behavior at net-istio set env var ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UI to true. 
This will be available by default in future releases.

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.

codecov · 2022-06-23T15:36:19Z

Codecov Report

Merging #862 (2307b89) into main (00505bc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #862   +/-   ##
=======================================
  Coverage   81.39%   81.39%           
=======================================
  Files          18       18           
  Lines        1172     1172           
=======================================
  Hits          954      954           
  Misses        174      174           
  Partials       44       44

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00505bc...2307b89. Read the comment docs.

skonto · 2022-06-24T08:55:53Z

config/300-controller.yaml

@@ -48,6 +48,8 @@ spec:
              value: "knative.dev/samples"
            - name: KOURIER_GATEWAY_NAMESPACE
              value: "kourier-system"
+            - name: ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UID
+              value: "true"


Will revert back to false, tests pass.

skonto · 2022-07-01T13:58:19Z

Tested in: knative/serving#13058

nak3 · 2022-07-05T02:29:08Z

@skonto Could you update the Release Note and Docs by s/net-istio/net-kourier?

nak3 · 2022-07-05T02:30:27Z

pkg/reconciler/ingress/controller.go

 	serviceinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/service"
+	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
+


Can you remove this blank line?

nak3 · 2022-07-05T02:47:32Z

test/generate-upstream-cert.sh


 kubectl create -n ${TEST_NAMESPACE} secret tls server-certs \
    --key="${out_dir}"/tls.key \
-    --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f -
+    --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | \
+    sed  '/^metadata:/a\ \ labels: {"networking.internal.knative.dev/certificate-uid":"test-id"}' | kubectl apply -f -


I think we don't need to add the label to the server-certs. It is used by users' namespace and Kourier does not need to be reconciled when they updated them.

(If users started using a server cert signed by a different CA, Kourier needs to be reconciled to load the new CA but it is triggered by knative-serving-certs(=CA) change.)

With said, the test will fail without this label? 😅

Kourier reconciles a related ingress referencing that secret through no? Afaik it does fail I can try once more without it.

Yes for the CA secret (=knative-serving-certs) , but should be no for the server cert (=server-certs).
But the test failed? Hmm... I misunderstood something. I will look into it as well.

Let me try it first. I might remember wrongly.

Ok removed the label and switched on filtering.

@nak3 Tests passed you were right,reverting the default value for the env var.

nak3 · 2022-07-05T10:34:18Z

/lgtm
/approve

Thank you!

knative-prow · 2022-07-05T10:34:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3, 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:

~~OWNERS~~ [nak3,skonto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* support secret filtering informer * fixes * revert env var default val

skonto force-pushed the filter_informer_fix branch from afc1efb to 8f9d892 Compare June 23, 2022 15:33

skonto force-pushed the filter_informer_fix branch from 8f9d892 to f2d8d3c Compare June 24, 2022 08:14

skonto commented Jun 24, 2022

View reviewed changes

skonto mentioned this pull request Jun 24, 2022

[TEST-ONLY] Test filtering secret informer knative/serving#13058

Closed

skonto force-pushed the filter_informer_fix branch 3 times, most recently from 639d755 to 88bf41a Compare July 1, 2022 12:03

support secret filtering informer

dae609b

skonto force-pushed the filter_informer_fix branch from 88bf41a to dae609b Compare July 1, 2022 12:29

skonto changed the title ~~[wip] Support secret filtering informer~~ Support secret filtering informer Jul 1, 2022

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 1, 2022

skonto mentioned this pull request Jul 1, 2022

Add filtering secret informer label for internal-encryption knative/serving#13082

Merged

nak3 reviewed Jul 5, 2022

View reviewed changes

Stavros Kontopoulos added 2 commits July 5, 2022 11:53

fixes

49a2643

revert env var default val

2307b89

knative-prow bot assigned nak3 Jul 5, 2022

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2022

knative-prow bot merged commit c885488 into knative-extensions:main Jul 5, 2022

skonto pushed a commit to skonto/net-kourier that referenced this pull request Oct 4, 2022

Support secret filtering informer (knative-extensions#862)

f72c73b

* support secret filtering informer * fixes * revert env var default val

skonto mentioned this pull request Oct 4, 2022

[RELEASE-1.5][SRVKS-944] Support secret filtering informer openshift-knative/net-kourier#32

Merged

nak3 mentioned this pull request Aug 3, 2023

[WIP] Add filtering label to secret and enable secret informer filtering by default #1084

Closed

1 task

		serviceinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/service"
		filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"

Support secret filtering informer #862

Support secret filtering informer #862

Uh oh!

Conversation

skonto commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

knative-prow bot commented Jun 23, 2022

Changes

Uh oh!

codecov bot commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skonto Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto commented Jul 1, 2022

Uh oh!

nak3 commented Jul 5, 2022

Uh oh!

nak3 Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

nak3 Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

nak3 Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

skonto Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

nak3 Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

skonto Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

skonto Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

skonto Jul 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nak3 commented Jul 5, 2022

Uh oh!

knative-prow bot commented Jul 5, 2022

Uh oh!

Uh oh!

skonto commented Jun 23, 2022 •

edited

Loading

codecov bot commented Jun 23, 2022 •

edited

Loading

skonto Jun 24, 2022 •

edited

Loading

skonto Jul 5, 2022 •

edited

Loading