PeerAuthentication API docs wrong

[howardjohn]

https://istio.io/latest/docs/reference/config/networking/sidecar/#WorkloadSelector is linked from PA docs. This is wrong, PA has matchLabels, not labels

[hzxuzhonghu]

not only pa but all the security api has the same issue

[costinm]

Can we start fixing the issue instead of making it worse ? Users don't enjoy an API where each team uses a different keyword.

On Tue, Oct 20, 2020 at 2:04 AM Zhonghu Xu [email protected] wrote:

not only pa but all the security api has the same issue

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/istio/api/issues/1700#issuecomment-712706063, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2WKWXAPDWIHTJGL5WLSLVHBPANCNFSM4SWM54UQ .

[Stono]

Just chiming in here, this does cause confusion. It's caught a few of my team out yesterday.

Sidecar:

    workloadSelector:
      labels:
        app: istio-test-app-1

PeerAuthentication:

    selector:
      matchLabels:
        app: istio-test-app-1

[yangminzhu]

the API is referring to the correct istio.type.v1beta1.WorkloadSelector https://github.com/istio/api/blob/0d3a960deddb1dead06c8bf35b9b01e4e95dbb0a/security/v1beta1/peer_authentication.proto#L124-L127 but the generated documentation is pointing to the wrong workload selector, @howardjohn do you know who is the owner of the doc generation or istio.io today? I'm not sure why it generates the wrong link here.

@costinm the networking API should use the generic WorkloadSelector instead of its own forked version, I don't see what could be changed in the security API.

[howardjohn]

There is no owner, I think the owners of certain APIs should fix the issues. Its probably not actually the tool thats broken but something in the proto. tool is in istio/tools if you need to change it, like https://github.com/istio/tools/pull/1261

[howardjohn]

Why should networking API change, wasn't it was using the selector first? We cannot break compatibility anyways, so best we could do is provide both most likely.

[yangminzhu]

There is no owner, I think the owners of certain APIs should fix the issues. Its probably not actually the tool thats broken but something in the proto. tool is in istio/tools if you need to change it, like istio/tools#1261

Thanks, Is there a way to test the istio/istio.io with local changes in istio/api? The location seems wrong but I'm not sure what would it look like if I change it something else: https://github.com/istio/api/blob/0d3a960deddb1dead06c8bf35b9b01e4e95dbb0a/type/v1beta1/selector.proto#L20

Why should networking API change, wasn't it was using the selector first? We cannot break compatibility anyways, so best we could do is provide both most likely.

the workload selector added in the common directory for the purpose to be used by all APIs, @liminw I thought there was a plan to use the common workload selector by changing sidecar.proto and other places when the common one is added? I'm not sure how the plan handles the compatibility though.

[GregHanson]

resolved by https://github.com/istio/api/pull/1705