-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add translation for GKE Ssl Policy #195
base: main
Are you sure you want to change the base?
Conversation
/assign @spencerhance |
bc9900c
to
2f53f39
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sawsa307 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2f53f39
to
a578e3d
Compare
Hi, just checking in, anything I can do to help with this? Ca this be reviewed by someone from GKE? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sawsa307! Mostly LGTM, just one nit.
@@ -883,6 +886,119 @@ func Test_convertToIR(t *testing.T) { | |||
}, | |||
expectedErrors: field.ErrorList{}, | |||
}, | |||
{ | |||
name: "ingress with a Frontend Config specifying Ssl Policy", | |||
ingresses: map[types.NamespacedName]*networkingv1.Ingress{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These kinds of tests can become really difficult to follow if you have to spell out the entirety of each manifest for each test case. I'd recommend moving towards a pattern where there's a base resource that each test case makes small modifications to. Here's an example of that pattern in Kubernetes: https://github.com/kubernetes/kubernetes/blob/80941e3e87d3909aa70e4f38f37b447f4376661f/pkg/apis/storage/validation/validation_test.go#L1741.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Just updated based on the example.
For now I only make sure in each test case, we modify the input ingress based on a template ingress, and I'm still keeping the expected output in details to make sure we compare with to detailed outputs.
a578e3d
to
e123362
Compare
e123362
to
b897953
Compare
b897953
to
99e893e
Compare
* Each test case will based on the same Ingress and Service template and make modification on the resources.
99e893e
to
3e0beb7
Compare
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adding a new feature for GKE Ingress and Gateway conversion.
Which issue(s) this PR fixes:
Adding more feature for #87.
Does this PR introduce a user-facing change?: