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.

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

Tests for encryption with Kourier local gateway #13263

Merged
merged 6 commits into from
Sep 5, 2022

Conversation

mgencur
Copy link
Contributor

@mgencur mgencur commented Aug 31, 2022

This PR tests encryption from user application to Kourier local gateway which is configured like this:

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-kourier
  namespace: knative-serving
data:
  cluster-cert-secret: "server-certs"

The internal encryption is enabled via:

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-network
  namespace: knative-serving
data:
  inernal-encryption: "true"

Proposed Changes

  • Create server cert/key for kourier-gateway and create Secret from them
  • Pass the Secret via config-kourier/cluster-cert-secret
  • Create CA cert for the client side to verify server connection
  • Enable TLS for the http client in httpproxy image, pass the CA cert to the httpproxy image

This is very similar to knative-extensions/net-kourier#909 , just with a different way to set up Secrets (through ytt)

Note: When SERVER_NAME is not passed the test fails (as expected) with the following error:

service_to_service_test.go:164: Failed to start endpoint of httpproxy: response: status: 502, body: x509: certificate is valid for knative.dev, not current-subroute-local-s-t-s-rzdwvscq.serving-tests.svc.c2971137168.local
        , headers: map[Content-Length:[122] Content-Type:[text/plain; charset=utf-8] Date:[Thu, 01 Sep 2022 12:14:34 GMT] Server:[envoy] X-Content-Type-Options:[nosniff] X-Envoy-Upstream-Service-Time:[51] Zipkin_trace_id:[d115549735c2bcfd746afbd075df8a3d]] did not pass checks: status = 502 502 Bad Gateway, want one of: [200]

Release Note


@knative-prow knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Aug 31, 2022
@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #13263 (430a698) into main (f517a90) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13263      +/-   ##
==========================================
- Coverage   86.58%   86.49%   -0.09%     
==========================================
  Files         196      196              
  Lines       14511    14526      +15     
==========================================
  Hits        12564    12564              
- Misses       1648     1663      +15     
  Partials      299      299              
Impacted Files Coverage Δ
pkg/autoscaler/statserver/server.go 77.77% <0.00%> (-1.49%) ⬇️
pkg/queue/sharedmain/main.go 0.61% <0.00%> (-0.01%) ⬇️
cmd/activator/main.go 0.00% <0.00%> (ø)
pkg/http/handler/timeout.go 84.76% <0.00%> (+1.32%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mgencur mgencur force-pushed the tls_cluster_local branch 2 times, most recently from a73fb47 to 79f6a0b Compare September 1, 2022 11:13
@mgencur mgencur changed the title [WIP] Tests for encryption with Kourier local gateway Tests for encryption with Kourier local gateway Sep 1, 2022
@knative-prow 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 Sep 1, 2022
@mgencur
Copy link
Contributor Author

mgencur commented Sep 1, 2022

/cc @nak3

@knative-prow knative-prow bot requested a review from nak3 September 1, 2022 12:28
@mgencur
Copy link
Contributor Author

mgencur commented Sep 1, 2022

/cherry-pick release-1.6
/cherry-pick release-1.7
Branches release-1.5 and release-1.4 need to be updated manually (there are conflicts).

@knative-prow-robot
Copy link
Contributor

@mgencur: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6
/cherry-pick release-1.7
Branches release-1.5 and release-1.4 need to be updated manually (there are conflicts).

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.

@mgencur
Copy link
Contributor Author

mgencur commented Sep 1, 2022

/cherry-pick release-1.7

@knative-prow-robot
Copy link
Contributor

@mgencur: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.7

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.

@mgencur
Copy link
Contributor Author

mgencur commented Sep 1, 2022

/retest

1 similar comment
@mgencur
Copy link
Contributor Author

mgencur commented Sep 2, 2022

/retest

@nak3
Copy link
Contributor

nak3 commented Sep 5, 2022

/lgtm
/approve

I think this cert creation process should be automated by:

  • Create the cert via test code instead of the bash script - test/config/tls/generate.sh or
  • Implement the cert creation/rotation in controller. Users will not need any extra operation.

But it would be alright to merge this for now.

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

knative-prow bot commented Sep 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgencur, nak3

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2022
@knative-prow knative-prow bot merged commit 6d3d676 into knative:main Sep 5, 2022
@knative-prow-robot
Copy link
Contributor

@mgencur: new pull request created: #13277

In response to this:

/cherry-pick release-1.6
/cherry-pick release-1.7
Branches release-1.5 and release-1.4 need to be updated manually (there are conflicts).

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.

@knative-prow-robot
Copy link
Contributor

@mgencur: new pull request created: #13278

In response to this:

/cherry-pick release-1.7

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.

mgencur added a commit to mgencur/serving-1 that referenced this pull request Sep 6, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests
mgencur added a commit to mgencur/serving-1 that referenced this pull request Sep 6, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services
mgencur added a commit to mgencur/serving-1 that referenced this pull request Sep 6, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services
openshift-merge-robot pushed a commit to openshift/knative-serving that referenced this pull request Sep 6, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests
mgencur added a commit to mgencur/serving-1 that referenced this pull request Sep 6, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services
openshift-merge-robot pushed a commit to openshift/knative-serving that referenced this pull request Sep 9, 2022
…ically (#1236)

* [RELEASE-v1.5] Add manifest patch for internal-tls to `openshift/release/artifacts` (#1202)

* Add secret to 1.5 CI yaml

* auto generated

* Support config to deploy internal certificates automatically (knative#13005)

* Add certificate reconciler for internal certs

* Fix cert path

* Temporary use local networking repo

* Support internal-encryption configuration

* Use const for cert name

* Fix lint

* rm blank line

* Drop unused variable

* Use one line style

* Use one line code

* Update net-kourier nightly

bumping knative.dev/net-kourier d758682...b9b1e8b:
  > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855)
  > 427434c bump kind and k8s versions in kind-e2e tests (# 859)

Signed-off-by: Knative Automation <automation@knative.team>

* Verify SecretPKKey as well

* Do not drop activator always in the path

* Comment about ctrl-ca suffix

Co-authored-by: Knative Automation <automation@knative.team>

* Update deps

* Enable internal-tls on ocp-tls (#1203)

* Enable internal-tls on OCP 4.8

* Use tls to match JOB name

* Add a target to enable internal-tls in Makefile (#1224)

* Add a target to enable internal-tls in Makefile

* Update CI template for internal-tls enabled

* Tests for encryption with Kourier local gateway (knative#13263)

* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services

* Enable tls tests for cluster-local Kourier gateway

* Need to create test resources including the test namespace first
before installing Knative so that applying
test/config/tls/cert-secret.yaml succeeds

* TMP: Enable tls in the standard e2e make target - test purposes

* Use knative-serving-ingress ns for deploying server-certs

* Deploy certificates at test phase

* Separate test and install of installing certs

* Wait for knative-serving-ingress to exist

* Revert "TMP: Enable tls in the standard e2e make target - test purposes"

This reverts commit 5bb3549.

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Co-authored-by: Knative Automation <automation@knative.team>
openshift-merge-robot pushed a commit to openshift/knative-serving that referenced this pull request Sep 9, 2022
* Enable internal-tls on ocp-tls (#1203)

* Enable internal-tls on OCP 4.8

* Use tls to match JOB name

* Add a target to enable internal-tls in Makefile (#1224)

* Add a target to enable internal-tls in Makefile

* Update CI template for internal-tls enabled

* Tests for encryption with Kourier local gateway (knative#13263)

* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services

* Enable tls tests for cluster-local Kourier gateway

* Need to create test resources including the test namespace first
before installing Knative so that applying
test/config/tls/cert-secret.yaml succeeds

* TMP: Enable tls in the standard e2e make target - test purposes

* Fix indentation

* Use knative-serving-ingress ns for deploying server-certs

* Deploy certificates at test phase

* Separate test and install of installing certs

* Wait for knative-serving-ingress to exist

* Revert "TMP: Enable tls in the standard e2e make target - test purposes"

This reverts commit 54fabb3.

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
mgencur added a commit to mgencur/serving-1 that referenced this pull request Sep 12, 2022
* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services
nak3 pushed a commit to openshift/knative-serving that referenced this pull request Sep 13, 2022
* Tests for encryption with Kourier local gateway (knative#13263)

* Generate Secrets

* Commit generated cert-secret.yaml

* httpproxy enables tls client

* httpproxy uses https when CA_CERT specified

* Pass CA_CERT and SERVER_NAME env variables properly to tests

* Avoid using cluster-local certificates for external services

* Enable tls tests for cluster-local Kourier gateway

* Need to create test resources including the test namespace first
before installing Knative so that applying
test/config/tls/cert-secret.yaml succeeds

* Use knative-serving-ingress ns for deploying server-certs

* Deploy certificates at test phase

* Separate test and install of installing certs

* Wait for knative-serving-ingress to exist

* Use yq write to replace namespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants