Create dangling ingress check #388
Conversation
|
Could you add e2e test? |
|
Yes we can. Adding a e2e test |
There was a problem hiding this comment.
Thanks for the contrib + adding the E2E tests so fast!
I would have one question regarding the definition of dangling ingress resource:
Currently, we define the ingress rule as dangling if the service name does not match.
However, for the ingress to work correctly, the service's port name / number must match as well. This can easily be misconfigured.
I saw this multiple times in the past be the culprit of the ingress not working correctly, so I do see the opportunity here to improve this as well with the new check during linting.
I'd suggest we add this to the dangling check as well. Then, we will check that both the service name match as well as defined port (either via its name / number).
| Key: templateKey, | ||
| Description: "Flag ingress which do not match any service", | ||
| SupportedObjectKinds: config.ObjectKindsDesc{ | ||
| ObjectKinds: []string{objectkinds.Service}, |
There was a problem hiding this comment.
Shouldn't this be objectkinds.Ingress?
There was a problem hiding this comment.
yes it should be
| if _, ok := selectors[serviceName]; ok { | ||
| delete(selectors, serviceName) | ||
| if len(selectors) == 0 { | ||
| // Found the all! |
There was a problem hiding this comment.
super nit:
| // Found the all! | |
| // Found them all! |
| } | ||
|
|
||
| func getSelectorsFromIngress(ingress *networkingV1.Ingress) map[string]bool { | ||
| selectors := map[string]bool{} |
There was a problem hiding this comment.
I'd favor map[string]struct{} and setting selectors[s] = struct{}{} subsequently:
| selectors := map[string]bool{} | |
| selectors := map[string]struct{} |
|
I think it's a great idea to check the ports as well. Update the PR for that. |
docs/generated/checks.md
Outdated
|
|
||
| **Description**: Indicates when ingress do not have any associated services. | ||
|
|
||
| **Remediation**: Confirm that your ingress's backend correctly matches the name on one of your services. |
There was a problem hiding this comment.
| **Remediation**: Confirm that your ingress's backend correctly matches the name on one of your services. | |
| **Remediation**: Confirm that your ingress's backend correctly matches the name and port on one of your services. |
| @@ -0,0 +1,7 @@ | |||
| name: "dangling-ingress" | |||
| description: "Indicates when ingress do not have any associated services." | |||
| remediation: "Confirm that your ingress's backend correctly matches the name on one of your services." | |||
There was a problem hiding this comment.
| remediation: "Confirm that your ingress's backend correctly matches the name on one of your services." | |
| remediation: "Confirm that your ingress's backend correctly matches the name and port on one of your services." |
|
Thanks for the approval! Excited to be able to use this check I've been running into issues around this a lot lately. |
Creating a check for dangling ingresses. I think it will fit in well with the other rules that already exist in this tool. Have been seeing these pop up from time to time in the projects I've been working on and would love to be able to catch them at lint time.
I punted on dealing with resources that ingress can point to an only check for services, but could add that logic in if need be. My assumption was this would cover almost all of the use cases.
Closes #387