fix(httpProxy): drop status==valid filter

[nefelim4ag]

Description
According to the contour design docs: conflict, status
The status field can be invalid in many different situations, even not directly related to the httpProxy definition.
Also, most of them are not fatal and contour will try to serve traffic by other rules, even if one of the routing paths is broken.

As an example we currently working on defining an API gateway based on HTTPProxy, and a situation can happen when the service underlines gateway, can be deleted or renamed accidentally (that situation will lead to downtime because of external-dns filter out "invalid" definitions and drop our DNS record - this is the main issue here).

There is an example of an invalid state, where the contour can send traffic by other routes, but external-dns will delete the record because of only "valid" filtering.

status:
  conditions:
    - errors:
        - message: >-
            Spec.Routes unresolved service reference: service
            "app-ns/ab-tests1" not found
  currentStatus: invalid
  description: At least one error present, see Errors for details
  loadBalancer:
    ingress:
      - hostname: lpfk.example.com

In the parse function endpointsFromHTTPProxy we only read three fields:

1. metadata.annotations
2. spec.virtualhost.fqdn (which we check for emptiness and it can't be empty in normal circumstances)
3. status.loadBalancer.ingress - controlled by contour and validated/defined by contour pkg.

kubectl -n app-ns apply --force -f ./broken.httpProxy.yaml
The HTTPProxy "broken-proxy" is invalid: spec.virtualhost.fqdn: Required value

I suggest dropping valid/invalid filtering completely, another ingress (istio, gloo & etc) resources just don't have that field and if ingress itself exists - a record must be created. That is expected behavior.
Behavior where we check the internal state of objects, like senseless but syntax valid objects, missing services, tls - is not external-dns part of the job.

Thanks!

Checklist

• Unit tests updated
• End user documentation updated - not applicable

[k8s-ci-robot]

Hi @nefelim4ag. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[mloiseleur]

@josephglanville As the author of this provider, would you please share what you think about this PR on contour provider ?

[josephglanville]

I think this idea is sound, decoupling the validity of the HTTPProxy from record creation makes sense.

Unfortunately I don't remember my original motivations behind checking status, it's been a while since I have looked at this.

[mloiseleur]

Thanks !
/ok-to-test
/lgtm

[nefelim4ag]

JFYI:
Lint is in hang state, already 1d+, I'm not sure that it will be auto-retried eventually
Build
Started 1d 9h 35m 36s ago

[mloiseleur]

@nefelim4ag You'll need to git commit --amend --no-edit and git push --force in order to refresh this.

[szuecs]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: szuecs

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:

• OWNERS [szuecs]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[nefelim4ag]

Hi, @mloiseleur can you add lgtm label, again, please?

[szuecs]

/lgtm