Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect schema for responseClasses.condition.not in service profile #11483

Closed
Hexcles opened this issue Oct 13, 2023 · 3 comments · Fixed by #11510 or #11543
Closed

Incorrect schema for responseClasses.condition.not in service profile #11483

Hexcles opened this issue Oct 13, 2023 · 3 comments · Fixed by #11510 or #11543

Comments

@Hexcles
Copy link

Hexcles commented Oct 13, 2023

What is the issue?

According to documentation https://linkerd.io/2.14/reference/service-profiles/#response-match, responseClasses.condition.not should take a single response match:

However, I have to use the following to work

      responseClasses:
        - condition:
            not:
              - status:
                  min: 521
          isFailure: false

which suggests not takes a list instead of a single response match, which seems incorrect.

How can it be reproduced?

Put this snippet into any service profile:

      responseClasses:
        - condition:
            not:
              status:
                min: 521
          isFailure: false

Logs, error output, etc

│ Required type Tuple[DynamicPseudoType], but got
│ Object["status":Object["min":Number]]
│ ...at attribute:
│ object.spec.routes[1].responseClasses[1].condition.not

output of linkerd check -o short

N/A

Environment

  • k8s 1.24
  • EKS
  • Linux
  • linkerd 2.14.1

Possible solution

No response

Additional context

No response

Would you like to work on fixing this bug?

None

@Hexcles Hexcles added the bug label Oct 13, 2023
@Hexcles
Copy link
Author

Hexcles commented Oct 13, 2023

Actually, if I try to apply

      responseClasses:
        - condition:
            not:
              - status:
                  min: 521
          isFailure: false

I'd get

│ admission webhook "linkerd-sp-validator.linkerd.io" denied the request:
│ failed to validate ServiceProfile: error unmarshaling JSON: while decoding
│ JSON: json: cannot unmarshal array into Go struct field
│ ResponseMatch.spec.routes.responseClasses.condition.not of type
│ v1alpha2.ResponseMatch

so looks like there's a mismatch between the validation during terraform plan and apply

@kflynn
Copy link
Member

kflynn commented Oct 18, 2023

Hey @Hexcles -- yeah, this looks like a legitimate bug, let me see where we can fit this in...

@adleong adleong self-assigned this Oct 19, 2023
@hawkw hawkw mentioned this issue Oct 19, 2023
hawkw added a commit that referenced this issue Oct 19, 2023
## edge-23.10.3

This edge release fixes issues in the proxy and destination controller which can
result in Linkerd proxies sending traffic to stale endpoints. In addition, it
contains other bugfixes and updates dependencies to include patches for the
security advisories [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3 and GHSA-c827-hfw6-qwvm.

* Fixed an issue where the Destination controller could stop processing
  changes in the endpoints of a destination, if a proxy subscribed to that
  destination stops reading service discovery updates. This issue results in
  proxies attempting to send traffic for that destination to stale endpoints
  ([#11483], fixes [#11480], [#11279], and [#10590])
* Fixed a regression introduced in stable-2.13.0 where proxies would not
  terminate unused service discovery watches, exerting backpressure on the
  Destination controller which could cause it to become stuck
  ([linkerd2-proxy#2484] and [linkerd2-proxy#2486])
* Added `INFO`-level logging to the proxy when endpoints are added or removed
  from a load balancer. These logs are enabled by default, and can be disabled
  by [setting the proxy log level][proxy-log-level] to
  `warn,linkerd=info,linkerd_proxy_balance=warn` or similar
  ([linkerd2-proxy#2486])
* Fixed a regression where the proxy rendered `grpc_status` metric labels as a
  string rather than as the numeric status code ([linkerd2-proxy#2480]; fixes
  [#11449])
* Added missing `imagePullSecrets` to `linkerd-jaeger` ServiceAccount ([#11504])
* Updated the control plane's dependency on the `golang.google.org/grpc` Go
  package to include patches for [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3 ([#11496])
* Updated dependencies on `rustix` to include patches for GHSA-c827-hfw6-qwvm
  ([linkerd2-proxy#2488] and [#11512]).

[#10590]: #10590
[#11279]: #11279
[#11483]: #11483
[#11449]: #11449
[#11480]: #11480
[#11504]: #11504
[#11504]: #11512
[linkerd2-proxy#2480]: linkerd/linkerd2-proxy#2480
[linkerd2-proxy#2484]: linkerd/linkerd2-proxy#2484
[linkerd2-proxy#2486]: linkerd/linkerd2-proxy#2486
[linkerd2-proxy#2488]: linkerd/linkerd2-proxy#2488
[proxy-log-level]: https://linkerd.io/2.14/tasks/modifying-proxy-log-level/
[CVE-2023-44487]: GHSA-qppj-fm5r-hxr3
adleong added a commit that referenced this issue Oct 20, 2023
Fixes #11483

Service profile's response class schema indicates that a `not` response match should be an array.  This is incorrect and parsing of the response class will fail if an array is provided.  

Update the schema to properly indicate that `not`'s value should be an object.

Signed-off-by: Alex Leong <alex@buoyant.io>
@kflynn
Copy link
Member

kflynn commented Oct 26, 2023

(This fix didn't quite make stable-2.14.2 but should be out in an edge release shortly!)

mateiidavid added a commit that referenced this issue Oct 26, 2023
This stable release fixes issues in the proxy and Destination controller which
can result in Linkerd proxies sending traffic to stale endpoints. In addition,
it contains a bug fix for profile resolutions for pods bound on host ports and
includes patches for security advisory [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3

* Control Plane
  * Fixed an issue where the Destination controller could stop processing
    changes in the endpoints of a destination, if a proxy subscribed to that
    destination stops reading service discovery updates. This issue results in
    proxies attempting to send traffic for that destination to stale endpoints
    ([#11483], fixes [#11480], [#11279], [#10590])
  * Fixed an issue where the Destination controller would not update pod
    metadata for profile resolutions for a pod accessed via the host network
    (e.g. HostPort endpoints) ([#11334])
  * Addressed [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3 by upgrading several
    dependencies (including Go's gRPC and net libraries)

* Proxy
  * Fixed a regression where the proxy rendered `grpc_status` metric labels as
    a string rather than as the numeric status code ([linkerd2-proxy#2480];
    fixes [#11449])
  * Fixed a regression introduced in stable-2.13.0 where proxies would not
    terminate unusred service discovery watches, exerting backpressure on the
    Destination controller which could cause it to become stuck
    ([linkerd2-proxy#2484])

[#10590]: #10590
[#11279]: #11279
[#11483]: #11483
[#11480]: #11480
[#11334]: #11334
[#11449]: #11449
[CVE-2023-44487]: GHSA-qppj-fm5r-hxr3
[linkerd2-proxy#2480]: linkerd/linkerd2-proxy#2480
[linkerd2-proxy#2484]: linkerd/linkerd2-proxy#2484

Signed-off-by: Matei David <matei@buoyant.io>
mateiidavid added a commit that referenced this issue Oct 26, 2023
This stable release fixes issues in the proxy and Destination controller which
can result in Linkerd proxies sending traffic to stale endpoints. In addition,
it contains a bug fix for profile resolutions for pods bound on host ports and
includes patches for security advisory [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3

* Control Plane
  * Fixed an issue where the Destination controller could stop processing
    changes in the endpoints of a destination, if a proxy subscribed to that
    destination stops reading service discovery updates. This issue results in
    proxies attempting to send traffic for that destination to stale endpoints
    ([#11483], fixes [#11480], [#11279], [#10590])
  * Fixed an issue where the Destination controller would not update pod
    metadata for profile resolutions for a pod accessed via the host network
    (e.g. HostPort endpoints) ([#11334])
  * Addressed [CVE-2023-44487]/GHSA-qppj-fm5r-hxr3 by upgrading several
    dependencies (including Go's gRPC and net libraries)

* Proxy
  * Fixed a regression where the proxy rendered `grpc_status` metric labels as
    a string rather than as the numeric status code ([linkerd2-proxy#2480];
    fixes [#11449])
  * Fixed a regression introduced in stable-2.13.0 where proxies would not
    terminate unusred service discovery watches, exerting backpressure on the
    Destination controller which could cause it to become stuck
    ([linkerd2-proxy#2484])

[#10590]: #10590
[#11279]: #11279
[#11483]: #11483
[#11480]: #11480
[#11334]: #11334
[#11449]: #11449
[CVE-2023-44487]: GHSA-qppj-fm5r-hxr3
[linkerd2-proxy#2480]: linkerd/linkerd2-proxy#2480
[linkerd2-proxy#2484]: linkerd/linkerd2-proxy#2484

---------

Signed-off-by: Matei David <matei@buoyant.io>
Co-authored-by: Alejandro Pedraza <alejandro@buoyant.io>
Co-authored-by: Oliver Gould <ver@buoyant.io>
mateiidavid added a commit that referenced this issue Oct 27, 2023
This edge release includes a fix for the `ServiceProfile` CRD resource schema.
The schema incorrectly required `not` response matches to be arrays, while the
in-cluster validator parsed `not` response matches as objects. In addition, an
issues has been fixed in `linkerd profile`. When used with the `--open-api`
flag, it would not strip trailing slashes when generating a resource from
swagger specifications.

* Fixed an issue where trailing slashes wouldn't be stripped when generating
  `ServiceProfile` resources through `linkerd profile --open-api` ([#11519])
* Fixed an issue in the `ServiceProfile` CRD schema. The schema incorrectly
  required that a `not` response match should be an array, which the service
  profile validator rejected since it expected an object. The schema has been
  updated to properly indicate that `not` values should be an object ([#11510];
  fixes [#11483])
* Improved logging in the destination controller by adding the client pod's
  name to the logging context. This will improve visibility into the messages
  sent and received by the control plane from a specific proxy ([#11532])
* Fixed an issue in the destination controller where the metadata API would not
  initialize a `Job` informer. The destination controller uses the metadata API
  to retrieve `Job` metadata, and relies mostly on informers. Without an
  initialized informer, an error message would be logged, and the controller
  relied on direct API calls ([#11541]; fixes [#11531])

[#11541]: #11532
[#11532]: #11532
[#11531]: #11531
[#11519]: #11519
[#11510]: #11510
[#11483]: #11483

Signed-off-by: Matei David <matei@buoyant.io>
mateiidavid added a commit that referenced this issue Oct 27, 2023
This edge release includes a fix for the `ServiceProfile` CRD resource schema.
The schema incorrectly required `not` response matches to be arrays, while the
in-cluster validator parsed `not` response matches as objects. In addition, an
issues has been fixed in `linkerd profile`. When used with the `--open-api`
flag, it would not strip trailing slashes when generating a resource from
swagger specifications.

* Fixed an issue where trailing slashes wouldn't be stripped when generating
  `ServiceProfile` resources through `linkerd profile --open-api` ([#11519])
* Fixed an issue in the `ServiceProfile` CRD schema. The schema incorrectly
  required that a `not` response match should be an array, which the service
  profile validator rejected since it expected an object. The schema has been
  updated to properly indicate that `not` values should be an object ([#11510];
  fixes [#11483])
* Improved logging in the destination controller by adding the client pod's
  name to the logging context. This will improve visibility into the messages
  sent and received by the control plane from a specific proxy ([#11532])
* Fixed an issue in the destination controller where the metadata API would not
  initialize a `Job` informer. The destination controller uses the metadata API
  to retrieve `Job` metadata, and relies mostly on informers. Without an
  initialized informer, an error message would be logged, and the controller
  relied on direct API calls ([#11541]; fixes [#11531])

[#11541]: #11532
[#11532]: #11532
[#11531]: #11531
[#11519]: #11519
[#11510]: #11510
[#11483]: #11483

Signed-off-by: Matei David <matei@buoyant.io>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants