Check ingress rule contains HTTP paths #2361
Conversation
k8s-ci-robot
added
cncf-cla: yes
Codecov Report
@@ Coverage Diff @@
## master #2361 +/- ##
=========================================
+ Coverage 39.11% 39.21% +0.1%
=========================================
Files 73 73
Lines 5203 5205 +2
=========================================
+ Hits 2035 2041 +6
+ Misses 2878 2876 -2
+ Partials 290 288 -2
Continue to review full report at Codecov.
|
|
/assign @ElvinEfendi |
|
@aledbf: GitHub didn't allow me to assign the following users: ElvinEfendi. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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. |
|
/assign @antoineco |
|
@aledbf: GitHub didn't allow me to assign the following users: antoineco. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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. |
| @@ -598,6 +598,9 @@ func (s k8sStore) ListIngresses() []*extensions.Ingress { | |||
| continue | |||
| } | |||
| for ri, rule := range ing.Spec.Rules { | |||
| if rule.HTTP == nil { | |||
| continue | |||
| } | |||
There was a problem hiding this comment.
Is there no valid case for the ingress to not have any rule.HTTP defined? I see that below we create a default path when it does not exist, why do we not do the similar thing for rule.HTTP?
There was a problem hiding this comment.
@ElvinEfendi when you only define a backend in the ingress without rules. This acts as / as the path
There was a problem hiding this comment.
Are you referring to something like
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: test-ingress
spec:
backend:
serviceName: testsvc
servicePort: 80
? If so then in the case ing.Spec.Rules would be nil too no?
There was a problem hiding this comment.
okay, if that's the case then we will still be seeing nil pointer error at line 600
There was a problem hiding this comment.
? If so then in the case ing.Spec.Rules would be nil too no?
No because ing.Spec.Rules is not a pointer.
|
/assign @ElvinEfendi |
|
@aledbf: GitHub didn't allow me to assign the following users: ElvinEfendi. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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. |
|
/assign @ElvinEfendi |
|
How hard would it be to write a regression test for this? |
|
@ElvinEfendi I will add one after merging this PR and #2382 (in a different PR) |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi 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 |
|
the test coverage for this change is introduced at #2399 |
fixes #2357