Add TransparentProxy to Service and ProxyDefaults #914
Conversation
thisisnotashwin
force-pushed
the
cluster-resource
branch
from
36c6a73 to
0bc8ba9
Compare
thisisnotashwin
force-pushed
the
cluster-resource
branch
3 times, most recently
from
57a6869 to
962c3f2
Compare
thisisnotashwin
force-pushed
the
cluster-resource
branch
8 times, most recently
from
88bbff9 to
8e33469
Compare
thisisnotashwin
changed the title
thisisnotashwin
force-pushed
the
cluster-resource
branch
from
8e33469 to
9cb2a41
Compare
thisisnotashwin
force-pushed
the
cluster-resource
branch
from
9cb2a41 to
7800563
Compare
thisisnotashwin
requested review from
kschoche,
a team and
ishustava
and removed request for
a team
| transparentProxy: | ||
| description: TransparentProxy controls configuration specific to proxies in transparent mode. | ||
| properties: | ||
| outboundListenerPort: | ||
| description: The port of the listener where outbound application traffic is being redirected to. | ||
| type: integer | ||
| type: object |
There was a problem hiding this comment.
wondering if our docs should say that we don't support these?
There was a problem hiding this comment.
It doesnt say it in the docs but we do have a webhook that rejects any CRD that has this set with an error message that says: Please you annotation `
I went back and forth on this a little bit and decided to only use the webhook because I figured it would be confusing for the docs to say that it wasn't supported. Do you think it makes sense to also have it in the docs?
There was a problem hiding this comment.
Hmm yeah, I can see that. Let's keep it as is then.
There was a problem hiding this comment.
I added this as a part of the commit that adds Mode and UpstreamConfigs to ServiceDefaults. Will update these CRDs as a part of their equivalent helm commit if that is ok?
| github.com/hashicorp/consul/api v1.4.1-0.20210415000851-62fcf1ff17cd | ||
| github.com/hashicorp/consul/sdk v0.7.0 |
There was a problem hiding this comment.
not sure why this change is needed.
There was a problem hiding this comment.
We don't strictly need it for the acceptance tests here, but this ensure the acceptance tests run against a version of Consul that does explicitly support the TransparentProxy field, even though we do not set it or should allow it to be set. It was done more from a correctness perspective.
There was a problem hiding this comment.
got it, thanks for clarifying!
- Update webhook versions to only support v1
thisisnotashwin
force-pushed
the
cluster-resource
branch
from
7800563 to
46beda7
Compare
Changes proposed in this PR:
How I've tested this PR: Acceptance tests passed.
How I expect reviewers to test this PR: Code Review
Checklist: