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

test: update certs for TestHTTPSIngress #4261

Closed
wants to merge 1 commit into from

Conversation

randmonkey
Copy link
Contributor

@randmonkey randmonkey commented Jul 3, 2023

What this PR does / why we need it:

The certificates in tlsPairs used in test cases TestHTTPSIngress and TestTCPIngressTLSPassthrough are expired, and the SANs inside does not match the hostname we used. This causes TLS failure against kong gateway image kong/kong-gateway-dev:2.8.4.2-rc1. To prevent the failures, we generated new certificates, and moved them into a single file.

Which issue this PR fixes:

fixes #4257

Special notes for your reviewer:

In this PR, we generated new certificates with the following settings:
tlsPairFooExample:

[ req ]
default_bits = 2048
prompt = no
default_md = sha256
req_extensions = req_ext
distinguished_name = dn

[ dn ]
C = US
ST = California
L = San Fransisco
O = Kong Inc
OU = team kubernetes
CN = foo.example

[ req_ext ]
subjectAltName = @alt_names

[ alt_names ]
DNS.1 = foo.example

tlsPairBarExample:

[ req ]
default_bits = 2048
prompt = no
default_md = sha256
req_extensions = req_ext
distinguished_name = dn

[ dn ]
C = US
ST = California
L = San Fransisco
O = Kong Inc
OU = team kubernetes
CN = bar.example

[ req_ext ]
subjectAltName = @alt_names

[ alt_names ]
DNS.1 = bar.example

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey requested a review from a team as a code owner July 3, 2023 06:00
@randmonkey randmonkey force-pushed the test/update_https_ingress_certs branch from aec0234 to 5602443 Compare July 3, 2023 06:16
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.2 ⚠️

Comparison is base (4f933ab) 62.9% compared to head (5602443) 62.7%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4261     +/-   ##
=======================================
- Coverage   62.9%   62.7%   -0.2%     
=======================================
  Files        154     154             
  Lines      17125   17125             
=======================================
- Hits       10773   10740     -33     
- Misses      5667    5699     +32     
- Partials     685     686      +1     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@programmer04
Copy link
Member

There is issue #4118 for unifying dealing with certificates in tests, you can see in it that in our tests in one place certificates are created for each run

@randmonkey
Copy link
Contributor Author

There is issue #4118 for unifying dealing with certificates in tests, you can see in it that in our tests in one place certificates are created for each run

@programmer04 So the suggested solution is to extract the generateSelfSignedCert function in #4058 into a general helper function?

@programmer04
Copy link
Member

I think it would be nice to have just one implementation for all tests. I saw that you're touching this code, so I linked the issue to spread awareness. Generating certs on the fly ensures that nothing unexpected is encoded in the cert blob, you have knobs to configure it. WDYT?

@pmalek
Copy link
Member

pmalek commented Jul 3, 2023

I think it would be nice to have just one implementation for all tests.

+1

@randmonkey
Copy link
Contributor Author

close this because we have #4271.

@randmonkey randmonkey closed this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update certificates used in integration tests
3 participants