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

Istio admission webhook fails when time formatting with odd number % #42749

Closed
liron-telemessage opened this issue Jan 10, 2023 · 6 comments · Fixed by #42778
Closed

Istio admission webhook fails when time formatting with odd number % #42749

liron-telemessage opened this issue Jan 10, 2023 · 6 comments · Fixed by #42778
Assignees

Comments

@liron-telemessage
Copy link

liron-telemessage commented Jan 10, 2023

Bug Description

Hi,
when trying to reformat a certificate expiration date I use format sting to create a new date format.
seen here:
https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#config-access-log-format-start-time
and using the sting formats seen here:
https://en.cppreference.com/w/cpp/io/manip/put_time
here is an example of my configuration:

  http:
  - match:
    - uri:
        prefix: /
    route:
    - destination:
        host: my-svc
        port:
          number: 443
      headers:
        request:
          add:
            X-Forwarded-For: '%DOWNSTREAM_REMOTE_ADDRESS%'
            X-SSL-Client-Cert-Exp: '%DOWNSTREAM_PEER_CERT_V_END(%b %d %H:%M:%S %Y %Z)%'
            X-SSL-Client-Cert-Issuer: '%DOWNSTREAM_PEER_ISSUER%'
            X-SSL-Client-Cert-Subject: '%DOWNSTREAM_PEER_SUBJECT%'

But when saving the configuration Istio admission webhook fails the configuration with the following error:
could not be patched: admission webhook "validation.istio.io" denied the request: configuration is invalid: single % not allowed. Escape by doubling to %% or encase Envoy variable name in pair of %

function code can be found here:
https://github.com/istio/istio/blob/master/pkg/config/validation/validation.go
detailed explanation can be found here:
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers.html#custom-request-response-headers
from what I can tell is that this validation is required for space within a URL phrase but should not be enforced on a time format as seen above

Thank you for taking a look at this

Version

istioctl
client version: 1.14.1
control plane version: 1.13.2
data plane version: 1.13.2 (7 proxies)

kubectl
Client Version: v1.24.1
Kustomize Version: v4.5.4
Server Version: v1.23.5

helm
v3.10.3+g835b733

Additional Information

No response

@howardjohn
Copy link
Member

Just to clarify, are you 100% sure it is valid in envoy? And escaping via %% is not needed?

@liron-telemessage
Copy link
Author

liron-telemessage commented Jan 11, 2023

100% sure, once I've disabled the validation hook it works as expected @howardjohn.
escaping using %% is NOT needed.

@zirain
Copy link
Member

zirain commented Jan 11, 2023

how can you use that on http headers?
that format should be used for accessLogFormat on https://istio.io/latest/docs/reference/config/istio.mesh.v1alpha1/?

@liron-telemessage
Copy link
Author

liron-telemessage commented Jan 11, 2023

@zirain just like that X-SSL-Client-Cert-Exp: '%DOWNSTREAM_PEER_CERT_V_END(%b %d %H:%M:%S %Y %Z)%'
(this is the format I need).
and it works as expected (only when I disabled the validation web hook).

@zirain
Copy link
Member

zirain commented Jan 11, 2023

yeah, I got what you wanted.

@zirain
Copy link
Member

zirain commented Jan 11, 2023

the following lines need improved

if strings.Count(value, "%")%2 != 0 {

@zirain zirain changed the title Istio admission webhook fails START_TIME formatting Istio admission webhook fails START_TIME formatting with odd number % Jan 11, 2023
@zirain zirain changed the title Istio admission webhook fails START_TIME formatting with odd number % Istio admission webhook fails when time formatting with odd number % Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants