Skip to content

Conversation

@Revolyssup
Copy link
Contributor

No description provided.

@Revolyssup Revolyssup marked this pull request as draft September 18, 2024 16:26
@Revolyssup Revolyssup changed the title feat: support SSL(DRAFT) feat: support SSL Sep 18, 2024
@Revolyssup
Copy link
Contributor Author

@AlinsRan Can you take a look at again at certificate extraction? I have taken reference from

cert, key, err := translation.ExtractKeyPair(s, true)

@Revolyssup Revolyssup requested a review from AlinsRan September 19, 2024 05:48
@Revolyssup
Copy link
Contributor Author

@AlinsRan Currently E2E tests are in dysfunctional state. So for the completion of this subtask, the passing tests need to be there or not?

@AlinsRan
Copy link
Contributor

@Revolyssup At present, e2e testing is mainly focused on basic functions.
We need to find the reason before we can consider skipping test cases. We cannot introduce other issues just because of a certain feature.

@AlinsRan
Copy link
Contributor

AlinsRan commented Sep 20, 2024

image

The test cases you wrote yourself are all incorrect. I suggest you run them locally:

make kind-e2e-test or make e2e-test

@Revolyssup
Copy link
Contributor Author

locally the tests passing. trying to fix the ci issue

@Revolyssup Revolyssup marked this pull request as ready for review September 22, 2024 15:12
@Revolyssup
Copy link
Contributor Author

Revolyssup commented Sep 22, 2024

@AlinsRan E2E test passing, need to pass conformance test

@AlinsRan
Copy link
Contributor

We also need a use case for accessing HTTPS based on HTTPRoute.

var skippedTestsForTraditionalRoutes = []string{
// TODO: Support ReferenceGrant resource
tests.HTTPRouteInvalidReferenceGrant.ShortName,
tests.GatewayWithAttachedRoutes.ShortName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why skip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One test in this was failing. Trying to pass it now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passed it

continue
}
secret := corev1.Secret{}
for _, ref := range listener.TLS.CertificateRefs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Need condition ref.kind == secret

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Snis: []string{},
}
name := listener.TLS.CertificateRefs[0].Name
secret := tctx.Secrets[types.NamespacedName{Namespace: ns, Name: string(ref.Name)}]
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@AlinsRan AlinsRan requested a review from nic-6443 September 27, 2024 01:29
@Revolyssup Revolyssup merged commit 418edc0 into release-v2-dev Sep 27, 2024
8 checks passed
@Revolyssup Revolyssup deleted the revolyssup/support/ssl branch September 27, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants