Skip to content

e2e: refactor TLS helpers #327

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

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

stlaz
Copy link
Collaborator

@stlaz stlaz commented Nov 26, 2024

  • simplification of the current certgen code
  • better separation of leaf and CA certs in kube CMs/Secrets

@stlaz stlaz added the sig-auth-acceptance issues created during review for sig-auth-acceptance label Nov 26, 2024
_, err = client.CoreV1().Secrets(ctx.Namespace).Create(context.TODO(), &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
},
Data: map[string][]byte{
"tls.crt": pem.EncodeToMemory(&pem.Block{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link

@alegacy alegacy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

...thought maybe there was an issue since the tests failed, but ran them on my system and they were fine.

@stlaz
Copy link
Collaborator Author

stlaz commented Nov 27, 2024

The test failed on KRP logging
I1126 13:37:03.550424 1 log.go:245] http: proxy error: dial tcp 10.96.218.31:8443: i/o timeout
That might be transient. The tests seemed to pass on my machine, too 👀

@stlaz stlaz self-assigned this Nov 27, 2024
@stlaz
Copy link
Collaborator Author

stlaz commented Nov 28, 2024

Looks like the same test failed again, that shouldn't happen. I'll have a look.

@stlaz
Copy link
Collaborator Author

stlaz commented Nov 28, 2024

Still not able to reproduce locally. Raising nginx loglevel to debug in CI.

Unify cert template generation. SKID and AKID should be properly
computed by Golang, no need to add them explicitly - unless we need
explicit SKID but that's not the case in our tests.
The previous design was mixing CA and leaf certificates, making it
easy to confuse them at place of use. Have all the leaf cryptomaterial
in a secret, and the trust in the CM to avoid those issues.
@stlaz stlaz force-pushed the e2e_cryptorefactor branch from afd6e94 to daccc4d Compare November 29, 2024 13:54
@stlaz stlaz force-pushed the e2e_cryptorefactor branch from 2959053 to 48dfc38 Compare December 2, 2024 11:10
@stlaz
Copy link
Collaborator Author

stlaz commented Dec 2, 2024

Looks like the test was missing assumptions on when it's safe to be run. I'll give it a couple respins to check the timing issue is now gone.

@ibihim ibihim merged commit ffd3bdd into brancz:sig-auth-acceptance Dec 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig-auth-acceptance issues created during review for sig-auth-acceptance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants