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

Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template #1296

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

miheer
Copy link
Contributor

@miheer miheer commented Dec 2, 2022

Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template
https://issues.redhat.com/browse/NE-982

@miheer miheer changed the title Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template WIP:Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template Dec 2, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2022
@miheer miheer force-pushed the headers branch 2 times, most recently from f984871 to dd5b6e5 Compare December 2, 2022 11:19
Comment on lines 32 to 33
This enhancement extends the IngressController logging API and router API to allow the user to set
capture of HTTP headers in the haproxy.config file. Using the new API, users may specify a list of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this enhancement have to do with logging? What is "the router API"? I think you can just say that the enhancement extends the IngressController API and the Route API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste error from your logging enhancement. BTW it was not ready for review. So, sorry for that!


This enhancement extends the IngressController logging API and router API to allow the user to set
capture of HTTP headers in the haproxy.config file. Using the new API, users may specify a list of
HTTP headers either via IngressController API or Route API that should be set in the haproxy.config file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The haproxy.config file is an implementation detail. Does the enhancement provide the ability to set HTTP request headers, HTTP response headers, or both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have asked this in the open questions. As per the RFE all the headers which customer need seem to response headers.

Comment on lines 43 to 46
As a cluster administrator, I need to configure an IngressController to set certain HTTP headers,
in order to satisfy compliance requirements.
To satisfy this use-case, the cluster administrator can either set the IngressController's or Router's spec.httpSetHeaders
field to set the required headers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific? Given the history of the RFE (in which various people have made requests about various HTTP headers) and the fact that you are modifying both the Route API and the IngressController API, I would expect multiple user stories.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 50 to 53
1. Enable the cluster administrator to specify HTTP headers in the IngressController CR Spec that should get set in the global section
of the haproxy.config.
2. Enable the developer or user to specify HTTP headers in the Route Spec that should get set in the backend section `default_backend openshift_default`.
of the haproxy.config.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is useful to specify whom you are enabling (cluster administrator or route owner). However, "set in the global section of the haproxy.config" and "set in the backend section default_backend openshift_default. of the haproxy.config" are implementation details (and I don't think the openshift_default backend is relevant for the implementation of the feature anyway). In this section, focus on the goal, not the implementation. What is the enhancement ultimately enabling the cluster admin or route owner to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


### Non-Goals

1. Admins or users are responsible for providing proper header values as inputs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you are saying here. Are you saying that validating input is not a goal (key word: "proper")? Are you saying that admins and users will be able to specify names but not values (key word: "values")?

Copy link
Contributor Author

@miheer miheer Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes validating HTTP header name and value is not our goal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

The values of `RouteSetHTTPHeaders` are comma-delimited lists of values of
the form `name:value`. (Colons and commas are not allowed in HTTP header
names, so their use unambiguously delimits values in the environment variable.)
The template translates these values into an appropriate `http-response set-header` stanzas.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the number agreement:

Suggested change
The template translates these values into an appropriate `http-response set-header` stanzas.
The template translates these values into the appropriate `http-response set-header` stanzas.

Beyond that, you're repeating a sentence that you used in another section. If you find yourself repeating some text, please either rework the text to avoid duplication or use signposting to improve the cohesion of the text. For example:

Suggested change
The template translates these values into an appropriate `http-response set-header` stanzas.
Similarly to the treatment of the `ROUTER_SET_HTTP_HEADERS` variable for global settings, the template translates the route-specific `RouteSetHTTPHeaders` values into the appropriate `http-response set-header` stanzas.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is no longer applicable. As from route we don't encode values.

`X-Frame-Options: DENY,X-XSS-Protection: 1;mode=block`, then the template adds the following stanzas to
the appropriate frontends:

```haproxy.config's backend section called default_backend openshift_default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to put this text outside the block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 259 to 260
[Code location](https://github.com/openshift/router/blob/master/images/router/haproxy/conf/haproxy-config.template#L608) where this
will be added.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a permalink, or delete the link if it isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been changed.

Comment on lines 17 to 18
creation-date: 2020-06-10
last-updated: 2020-06-10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dates need to be updated.

last-updated: 2020-06-10
tracking-link:
- "https://issues.redhat.com/browse/NE-982"
status: implemented
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line needs to be updated or deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Added TODO

Comment on lines 50 to 51
1. Enable the cluster administrator to specify HTTP headers in the IngressController CR Spec that should get set in the global section
of the haproxy.config.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how the headers can be set in the global section of the HAProxy config. http-request and http-response can only be set in frontend/listen/backend sections.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, the implementation details mention frontend section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, that is the reason we don't need add-header.

I don't understand. It looks like you just described what "set-header" and "add-header" do, but I don't see any explanation of why we don't need "add-header".

So, that is the reason we don't need add-header.

I don't understand. It looks like you just described what "set-header" and "add-header" do, but I don't see any explanation of why we don't need "add-header".

fixed it

Comment on lines 78 to 82
httpSetHeaders:
- name: X-Frame-Options
value: DENY
- name: X-XSS-Protection
value: 1;mode=block
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where exactly the headers specified on the ingresscontroller level should be set in HAproxy config? Will they be set on all the existing routes (backends in HAProxy config)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the visibility for the user? Should these headers be added to the route's status or to the route's spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec

[Code location](https://github.com/openshift/router/blob/master/images/router/haproxy/conf/haproxy-config.template#L608) where this
will be added.

### Risks and Mitigations
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering how this feature will co-exist with the OpenShift's Ingress HSTS policy which also sets a header (STS header) on the routes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed by a comment in the RFE:

Create an “immutable” OpenShift deny-list of headers that are known in advance to be problematic (e.g. HSTS)

https://issues.redhat.com/browse/RFE-464?focusedCommentId=20064927&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-20064927

Copy link
Contributor Author

@miheer miheer Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Miciah but won't it be a different epic ? Adding admission list will again need a API field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the router we can check the header for Strict-Transport-Security: in the route and then ignore it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would it be a different epic?

Going back to the comment I cited:

Create an “immutable” OpenShift deny-list of headers that are known in advance to be problematic (e.g. HSTS)

We don't need an API field for the immutable deny-list, which would include strict-transport-security.

The comment I cited also mentions the following:

Cluster Admins can delegate per-route controls (perhaps with an allow-list, or an attribute in the global ones above) to, for example, Project Admins

We would need a field in the IngressController API for that. If there's a reason this should be a separate epic, please give an argument. We can discuss it with PM if it really should be out of scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so the immutable deny list will exist in the code and will be documented. It'll include HSTS (as managed by the route's annotation) and potentially some other headers.

Copy link
Contributor Author

@miheer miheer Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation details added.

@miheer miheer force-pushed the headers branch 2 times, most recently from d235a03 to 2c8bcaa Compare December 14, 2022 13:20
Copy link
Contributor

@Miciah Miciah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I mostly like the direction the API is going in, and the goals and non-goals are more complete. I still have some questions, and some of my earlier comments still need to be addressed.


### User Stories

1. As a cluster administrator, I need to configure an IngressController to set/append certain response HTTP headers which are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. As a cluster administrator, I need to configure an IngressController to set/append certain response HTTP headers which are:
1. As a cluster administrator, I need to configure an IngressController to set/append certain HTTP response headers, such as the following:

Comment on lines 44 to 49
```conf
Strict-Transport-Security "max-age=31536000; includeSubdomains"
adding X-XSS-Protection "1; mode=block"
adding X-Content-Type-Options "nosniff"
adding X-Frame-Options "SAMEORIGIN"
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What syntax is this? I would expect either prose with a bulleted list or an excerpt of a syntactically valid HTTP response.

Why is "Strict-Transport-Security" in this example? In another comment thread, we are discussing a built-in deny-list, which would include this particular header (i.e. we would prohibit the cluster admin from specifying this header), so it seems to me like a bad example to use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we were not sure if deny list shall be added at that time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we didn't add the deny list, it would be better not to provide an example of something we would have denied. * grin *.

Comment on lines 50 to 51
in order to satisfy compliance requirements at global level i.e frontend fe_no_sni, frontend public, frontend fe_sni
sections of the haproxy.config.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User stories should focus on the user perspective; "frontend fe_no_sni, frontend public, frontend fe_sni" are implementation details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

Comment on lines 59 to 60
1. Enable the cluster administrator to specify HTTP headers in the IngressController CR Spec that should get set/append at the global
level for all backends which is not application specific.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to say whether the goals include customizing request or response headers. For now, you can go with "response headers" while you discuss the requirements with PM and other stakeholders.

Suggested change
1. Enable the cluster administrator to specify HTTP headers in the IngressController CR Spec that should get set/append at the global
level for all backends which is not application specific.
1. Enable the cluster administrator to specify HTTP response headers in the IngressController CR spec that should get set or appended at the global
level, i.e. for all routes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

Comment on lines 61 to 62
2. Enable the developer or user to specify HTTP headers in the Route Spec of Route(which is not a passthrough route) for their application
that should get set/append for sending to application's backends only.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "for sending to application's backends only"? This phrase makes it seem as if the goal were to allow customizing request headers (which get sent to the backend) and not to allow customizing response headers (which get sent back to to the client). I thought we definitely wanted to allow customizing response headers (such as the ones in your examples: x-xss-protection, x-content-type-options, and x-frame-options), and you have an open question about customizing request headers.

Suggested change
2. Enable the developer or user to specify HTTP headers in the Route Spec of Route(which is not a passthrough route) for their application
that should get set/append for sending to application's backends only.
2. Enable the route owner to specify HTTP response headers in the Route spec (when it is not a passthrough Route) for their application
that should get set or appended when forwarding a response to the client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


### Open Questions [optional]

- Do we need to set request headers ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise this with PM, and ask stakeholders on the RFE. If no one has asked for it, we can assume for now that customizing request headers is a non-goal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

### Open Questions [optional]

- Do we need to set request headers ?
- Do we need a delete option ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed


- Do we need to set request headers ?
- Do we need a delete option ?
- Do we a need admission list option ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was mentioned on the RFE. If you have concerns about this, please explain those concerns here and raise them with PM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no concern as such but it is like multiple feature requests combined in one epic.
I think will increase the number of points in the JIRA.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please describe in the EP what you mean by "admission list option" so that we can discuss it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are no longer creating an admission list. we have a immutable list of not allowed response headers

- Do we need to set request headers ?
- Do we need a delete option ?
- Do we a need admission list option ?
- Do we need to set load balance algo based on the header set ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why we would set the balance algorithm based on an HTTP header. Where does this question come from? Is there a particular header you have in mind? Is this question related to some statement or comment in the RFE, or some request someone has conveyed to you?

Copy link
Contributor Author

@miheer miheer Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@Miciah Miciah Dec 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the request is that if a route is annotated with haproxy.router.openshift.io/balance: hdr(x-foo), then OpenShift router should write balance hdr(x-foo) in haproxy.config. That's trivial to do, but it's orthogonal to customizing HTTP headers. I agree that it is out of scope for this enhancement and should be handled with a separate RFE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting load balancing algorithm based on headers is already mentioned in the Non-goals. Doesn't need to be in the open question. As a matter of fact I think this doesn't need to be mentioned at all since:

  1. We seem to go with the response only API while haproxy.router.openshift.io/balance: hdr()makes sense for the requests only.
  2. Even if we'd have the request API too that would only be a helper for the haproxy.router.openshift.io/balance: hdr() annotation, not enabler or prerequisite.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to me to list it as a non-goal because someone asked for it on the RFE. I don't know why the person who asked for it thought a feature for customizing headers would include "balance traffic based on the header", but since someone did think that, we might as well state explicitly that the enhancement does not include it.

Anyway, I think we are all in agreement that this is a non-goal, so @miheer, please add some text below this question in the EP to say that the question has been resolved, and that we have decided that this is a non-goal for this EP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miheer, reminder to add some text to the EP for this (and other) open questions. I'll also provide some examples in case that's helpful.

We have several DNS- and ingress-related EPs that present open questions (or questions that were previously open) in a Q&A style, such as the forward-dns-over-tls, cache-tuning, and lb-allowed-source-ranges EPs. The ip-interface-selection EP has a similar format, and the configurable-dns-loadbalancerservice EP similarly has an open question that has been "closed" with an explanation. We sometimes put open questions in the EP for follow-up, for example the route-subdomain EP. We sometimes write open questions with a note to document the issue, as in the haproxy-healthcheckinterval and tunable-router-kubelet-probes EPs. We also sometimes have an open question with a link to an issue tracker, as with the contour-operator EP. The console customize-add-page and customize-project-roles EPs do a really nice job of explaining the question with an example and quoting the decision that was made on the question (the only thing I would add would be a citation for the answer). The infrastructure-external-platform-type EP does a really nice job of listing open questions and "answered" questions with detailed answers and citations.

The purpose of writing down these open questions is to show some of the thought that went into the design so that people can read that these questions have been raised, discussed, and resolved (or possibly that they are still open questions that may warrant follow-up).

The next person who reads the EP cannot be expected to read through all the GitHub comments; it's your job to make sure that the EP itself has this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 367 to 368
- What shall be done if user sets a HSTS annotation in the route ? We can add a condition at [code location](https://github.com/openshift/router/blob/master/images/router/haproxy/conf/haproxy-config.template#L609)
to check if the annotation was set. If the annotation is set for hsts we will can use that value.
Copy link
Contributor

@Miciah Miciah Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, we should prohibit specifying strict-transport-security using the new API, for the following reasons:

Copy link
Contributor Author

@miheer miheer Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Miciah ! we can restrict this header by putting a condition in the haproxy.template. WDYT ?
The other way will be adding a API field for the admission list of headers.
Yes apiserver code refers ingress config object and verifies the hts annotation set in the route.

The annotation value is already validated in the template (deads2k wrote an admission plugin, but we
never merged it: openshift/kubernetes#1216).

Which annotation ? I tried to search for hsts but did not find anything related to it in the commit.

Copy link
Contributor Author

@miheer miheer Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openshift/kubernetes#1216 I think I can use it for verifying headers the ingress controller CR as per admission list I will set in the ingress config object.
For the verifying the headers mentioned in the route spec w.r.t to admission list mentioned in the ingress config we can follow the similar approach we for hsts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Miciah ! we can restrict this header by putting a condition in the haproxy.template. WDYT ?

No, we can do better than that; we can restrict it in route validation, or write a new admission plugin alongside the existing admission plugins.

The other way will be adding a API field for the admission list of headers.

Why would we add an API field? The list of prohibited headers is supposed to be an immutable list.

Yes apiserver code refers ingress config object and verifies the hts annotation set in the route.

Indeed, I found the correct link, and I have edited my earlier comment.

The annotation value is already validated in the template (deads2k wrote an admission plugin, but we
never merged it: openshift/kubernetes#1216).

Sorry, I mixed up PRs. That admission plugin for the annotation was merged: openshift/openshift-apiserver#224

Which annotation ? I tried to search for hsts but did not find anything related to it in the commit.

haproxy.router.openshift.io/hsts_header.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing this with someone from the API team, I understand that adding validation for spec.httpSetHeaders to the existing route validation code in library-go is the way to go. (The "admission list option" mentioned earlier in this EP might require an admission plugin though, depending on what you have in mind for that option.)

Copy link
Contributor Author

@miheer miheer Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Miciah

Why would we add an API field? The list of prohibited headers is supposed to be an immutable list.

I didn't get it regarding immutable lists. We will need the input of headers to be denied either from the route Spec or route Annotation right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think I got it that we will be hard coding some headers in the library-go validation for routes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get it regarding immutable lists. We will need the input of headers to be denied either from the route Spec or route Annotation right ?

No, "immutable" means that the user cannot change it.

i think I got it that we will be hard coding some headers in the library-go validation for routes.

Right, it can be hard-coded somewhere, and library-go is a reasonable place for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@miheer miheer force-pushed the headers branch 4 times, most recently from eb32a1a to 39ed8f9 Compare December 20, 2022 16:50
@miheer miheer changed the title WIP:Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template Enhancement Proposal for Adding HTTP header to HAProxy Without Customizing haproxy.config Template Dec 20, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 20, 2022
@miheer miheer force-pushed the headers branch 8 times, most recently from fe79bed to a80d527 Compare December 24, 2022 15:03
}
```

We will be using the exisiting API:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence needs to be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@alebedev87 alebedev87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some suggestions on Summary and Motivation chapters.

Comment on lines 32 to 35
This enhancement extends the IngressController API and Route API to allow the user to set(replace) or append
HTTP Response headers in the haproxy.config file. Using the new API, users may specify a list of
HTTP Response headers either via IngressController API or Route API that should be set(replace) or appended or deleted.
Creates an immutable(hardcoded) list of HTTP Response headers which can't be set(replace) or appended or deleted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This enhancement extends the IngressController API and Route API to allow the user to set(replace) or append
HTTP Response headers in the haproxy.config file. Using the new API, users may specify a list of
HTTP Response headers either via IngressController API or Route API that should be set(replace) or appended or deleted.
Creates an immutable(hardcoded) list of HTTP Response headers which can't be set(replace) or appended or deleted.
This enhancement extends the IngressController API and Route API to allow the user to set(replace), append or delete
HTTP response headers. Using the new API, users may specify a list of
HTTP response headers either via IngressController API or Route API that should be set(replaced), appended or deleted.
Also, this enhancement creates an immutable(hard-coded) list of HTTP response headers which can't be set(replaced), appended or deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 47 to 65
1. As a cluster administrator, I need to configure an IngressController to set(replace)/append certain HTTP Response headers which are:
- X-XSS-Protection "1; mode=block"
- X-Content-Type-Options "nosniff"
- X-Frame-Options "SAMEORIGIN"
in order to satisfy compliance requirements at global level to send them to all clients.
2. As a user/devops/developer, I need to configure Route to set/append HTTP Response headers which need to which
are mentioned above to send them to the client which has called the route.
3. To satisfy this use-case, the cluster administrator can set the IngressController's Spec
or the developer can set Router's `spec.httpDeleteHeaders` field to set(replace) or append the required headers.
4. As a cluster administrator, I need to configure an IngressController to delete certain HTTP Response headers which are:
- X-XSS-Protection
- X-Content-Type-Options
- X-Frame-Options
in order to satisfy compliance requirements at global level to not to send them to all clients.
5. As a user/devops/developer, I need to configure Route to delete HTTP Response headers which need to which
are mentioned above for not sending them to the client which has called the route.
6. To satisfy this use-case, the cluster administrator can set the IngressController's Spec
or the developer can set the Router's Spec with `spec.httpDeleteHeaders` field to delete the required headers by
mentioning the header names mentioned above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. As a cluster administrator, I need to configure an IngressController to set(replace)/append certain HTTP Response headers which are:
- X-XSS-Protection "1; mode=block"
- X-Content-Type-Options "nosniff"
- X-Frame-Options "SAMEORIGIN"
in order to satisfy compliance requirements at global level to send them to all clients.
2. As a user/devops/developer, I need to configure Route to set/append HTTP Response headers which need to which
are mentioned above to send them to the client which has called the route.
3. To satisfy this use-case, the cluster administrator can set the IngressController's Spec
or the developer can set Router's `spec.httpDeleteHeaders` field to set(replace) or append the required headers.
4. As a cluster administrator, I need to configure an IngressController to delete certain HTTP Response headers which are:
- X-XSS-Protection
- X-Content-Type-Options
- X-Frame-Options
in order to satisfy compliance requirements at global level to not to send them to all clients.
5. As a user/devops/developer, I need to configure Route to delete HTTP Response headers which need to which
are mentioned above for not sending them to the client which has called the route.
6. To satisfy this use-case, the cluster administrator can set the IngressController's Spec
or the developer can set the Router's Spec with `spec.httpDeleteHeaders` field to delete the required headers by
mentioning the header names mentioned above.
1. As a developer, I need to update IngressController and Route APIs to be be able to set/append/delete certain HTTP response headers.
2. As a cluster administrator, I need to configure an IngressController to set/append/delete certain HTTP response headers in order to satisfy compliance requirements at global level to send/not to send them to the client has called any route from IngressController's domain.
3. As a user, I need to configure Route to set/append/delete HTTP response headers to send/not send them to the client which has called the route.

I suppose that the implementation would need only 3 user stories: API change, the changes in the cluster-ingress-operator and router.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for writing, "As a developer, I need to update ___ API" as a user story? User stories are usually focused on end-user roles, such as cluster admin or project admin.

I do like simplifying the user stories where possible and simplifying the list so that it enumerates one user story per list item. It is helpful to provide the solution to the user story though. When writing an EP, I put the user story as a heading or italicized statement followed by one or more paragraphs explaining how the EP addresses the user story.

The user story should indicate who the end-user is (e.g. "cluster admin"), what the end user is trying to achieve (e.g. "delete the x-server response header in responses to client requests") and why ("so that I can avoid exposing information about internal systems to potentially malicious external actors"). Then you can explain how the EP solves that user story ("The cluster admin can configure the IngressController as shown in the following example yaml").

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for writing, "As a developer, I need to update ___ API" as a user story? User stories are usually focused on end-user roles, such as cluster admin or project admin.

I created such user stories myself, for technical debt for instance. I suppose it's in the interest of NetEdge engineer to define and implement the deny list. That's why I put Developer as the actor for the first user story.

The user story should indicate who the end-user is (e.g. "cluster admin"), what the end user is trying to achieve (e.g. "delete the x-server response header in responses to client requests") and why ("so that I can avoid exposing information about internal systems to potentially malicious external actors"). Then you can explain how the EP solves that user story ("The cluster admin can configure the IngressController as shown in the following example yaml").

Isn't the Workflow chapter for this purpose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for writing, "As a developer, I need to update ___ API" as a user story? User stories are usually focused on end-user roles, such as cluster admin or project admin.

I created such user stories myself, for technical debt for instance. I suppose it's in the interest of NetEdge engineer to define and implement the deny list. That's why I put Developer as the actor for the first user story.

I can see creating user stories in Jira for tech debt, and we sometimes note potential future follow-up work in the goals/non-goals, proposal, or implementation details/notes/constraints sections. However, I still believe that user stories in the enhancement should be focused on end users.

The user story should indicate who the end-user is (e.g. "cluster admin"), what the end user is trying to achieve (e.g. "delete the x-server response header in responses to client requests") and why ("so that I can avoid exposing information about internal systems to potentially malicious external actors"). Then you can explain how the EP solves that user story ("The cluster admin can configure the IngressController as shown in the following example yaml").

Isn't the Workflow chapter for this purpose?

I would use user stories for the high-level description of the problem ("As a ___, I want to ___, so that I can ___") and solution ("To accomplish ___, ___ can use the new ___ API/field/whatever as in the following example") and then document the detailed step-by-step procedure as a workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 69 to 75
1. Enable the cluster administrator to specify HTTP response headers in the IngressController CR spec that should get set or appended at the global
level, i.e. for all routes.
2. Enable the route owner to specify HTTP response headers in the Route spec (when it is not a passthrough Route) for their application
that should get set or appended when forwarding a response to the client.
3. Enable the cluster administrator to delete the HTTP response headers via IngressController CR.
4. Enable the developer to delete the HTTP response headers via RouteSpec.
5. This EP will set an immutable/hardcoded deny list of headers not allowed to set/append or deleted.
Eg- we should prohibit specifying strict-transport-security using the new API, for the following reasons:
HSTS is already configurable through a route annotation.
We already have an API for restricting HSTS in the cluster config.
Adding a second way to configure HSTS would significantly complicate this enhancement.
Copy link
Contributor

@alebedev87 alebedev87 Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Enable the cluster administrator to specify HTTP response headers in the IngressController CR spec that should get set or appended at the global
level, i.e. for all routes.
2. Enable the route owner to specify HTTP response headers in the Route spec (when it is not a passthrough Route) for their application
that should get set or appended when forwarding a response to the client.
3. Enable the cluster administrator to delete the HTTP response headers via IngressController CR.
4. Enable the developer to delete the HTTP response headers via RouteSpec.
5. This EP will set an immutable/hardcoded deny list of headers not allowed to set/append or deleted.
Eg- we should prohibit specifying strict-transport-security using the new API, for the following reasons:
HSTS is already configurable through a route annotation.
We already have an API for restricting HSTS in the cluster config.
Adding a second way to configure HSTS would significantly complicate this enhancement.
1. Enable the cluster administrator to specify HTTP response headers in the IngressController CR spec that should be set, appended or deleted at the global level, i.e. for all routes.
2. Enable the user to specify HTTP response headers in the Route spec (when it is not a passthrough Route) for their application
that should be set, appended or deleted when forwarding a response to the client.
3. Define an immutable/hard-coded deny list of headers which are not allowed to be set, appended or deleted.
Eg- we should prohibit specifying strict-transport-security using the new API, for the following reasons:
HSTS is already configurable through a route annotation.
We already have an API for restricting HSTS in the cluster config.
Adding a second way to configure HSTS would significantly complicate this enhancement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@alebedev87 alebedev87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments about the new API.

`IngressControllerSetHTTPHeaders` to `IngressControllerSpec`.

```go
type IngressControllerSpec struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IngressControllerSpec already has a field named HTTPHeaders which provides some parameters to manipulate the headers like adding the unique ID header or adjusting the header name's case.
For the sake of the logical regrouping, did you consider IngressControllerSpec.HTTPHeaders as the parent for the new API field(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// httpHeaders defines policy for HTTP headers.
//
// If this field is empty, the default values are used.
//
// +optional
HTTPHeaders *IngressControllerHTTPHeaders `json:"httpHeaders,omitempty"`

The goal of this seems to be different i.e to set policies. But we can add the new field under this. WDYT @Miciah ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using IngressControllerHTTPHeaders makes sense to me. Can we do that and still keep the new IngressController API and the new Route API consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 261 to 334
const (
// AppendHTTPHeader appends the header, preserving any existing header.
AppendHTTPHeader IngressControllerHTTPHeaderAction = "Append"
// SetOrReplaceHTTPHeader sets the header, removing any existing header.
SetOrReplaceHTTPHeader IngressControllerHTTPHeaderAction = "SetOrReplace"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand the purpose of the action field.
The API seems to be designed with the HAProxy config in mind: http-response set-header is supposed to be used for the set case and http-response del-header for the delete case. In which case the question of why Action is needed arises.
However if the API tries to be a little more generic then the delete action can also be added, simplifying the API (HTTPDeletHeaders nor needed anymore).

Copy link
Contributor Author

@miheer miheer Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Value field is mandatory for Set or Replace headers so we will need different API for delete headers as it not need a value field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think you have the point about the Value field. It's probably better to be validated by the OpenAPI, this may be clearer for the end user.
However, what about the actions? How do you plan to use them in the cluster-ingress-operator or router?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a discriminated union where IngressControllerHTTPHeaderAction would be the discriminator, "Delete" would be an empty union member, and "Set" would be union member with a "Value" field. "Replace" and "Append" could be expressed as separate union members or as a field of the "Set" union member.

Copy link
Contributor

@alebedev87 alebedev87 Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the delete as a union member:

// +union
type IngressControllerHTTPHeader struct {
  // +unionDiscriminator
  // +kubebuilder:validation:Required
  Action IngressControllerHTTPHeaderAction `json:"action,omitempty"`
  // +optional
  Set *IngressControllerSetHTTPHeader `json:"set,omitempty"`
  // +optional
  Delete *IngressControllerDeleteHTTPHeader `json:"delete,omitempty"`
}
apiVersion: operator.openshift.io/v1                                                                                                         
kind: IngressController
metadata:
  name: default
spec:
  httpHeaders:
    headerManipultation:
      response:
      - action: "Set"
        set:
          name: "X-Frame-Options"
          value: "DENY"
      - action: "Delete"
        delete:
          name: "X-Powered-By"

but I cannot quite understand how it can be an empty union member.
Does it mean that only Delete action will be specified?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Miciah for clarifying.
I'm almost fine with the discriminated union approach. It just feels a little verbose, especially on this part:

- action: "Set"
  name: "X-Frame-Options"
  set:  <-- this part
    value: "DENY"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm almost fine with the discriminated union approach. It just feels a little verbose

True, but it's a standard pattern and makes it clear which fields apply to which action.

Copy link
Contributor

@alebedev87 alebedev87 Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to bring this back but I don't think I'm 100% convinced that the discriminated union approach is better than the dedicated Set and Delete fields:

apiVersion: operator.openshift.io/v1                                                                                                         
kind: IngressController
metadata:
  name: default
spec:
  httpHeaders:
    headerManipultation:
      response:
        set:
        - name: "X-Frame-Options"
          value: "DENY"
        delete:
        - name: "X-Powered-By"

The API maybe a little less redundant for the discriminated union (Name field is common, no DeleteHeader type is needed) but is it more convenient to the end user? IMO it feels a little more verbose than the variant I showed above.
Btw don't we loose some validation using Name as an empty union member (same question)? it has to be optional while we need it required, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The discriminated union approach allows the API users to create an ordered list of the header manipulation instructions. This maybe more explicit expression of their intent as the dedicated set/delete fields approach would always raise the question in which order the two lists need to be translated into HAProxy configurations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw don't we loose some validation using Name as an empty union member (same question)? it has to be optional while we need it required, no?

For the record, I believe I answered the question here: #1296 (comment)

Name should not be a union member. You can use a inline field for the union type. See IngressControllerCaptureHTTPCookie (source) for an example.

Please let me know if that doesn't satisfactorily answer your question!

//
// +kubebuilder:validation:Optional
// +optional
HTTPSetHeaders RouteSetHTTPHeaders `json:"httpSetHeaders,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the question I asked about the IngressController API: what do you think about a field which regroup all the things which concern the headers? Similar to IngressControllerSpec.HTTPHeaders.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 396 to 430
const (
// AppendHTTPHeaderPolicy appends the header, preserving any existing header.
AppendHTTPHeader RouteHTTPHeaderPolicy = "Append"
// ReplaceHTTPHeaderPolicy sets the header, removing any existing header.
ReplaceHTTPHeaderPolicy RouteHTTPHeaderPolicy = "SetOrReplace"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as for the IngressController API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

**Cluster admin** is a human user responsible for deploying a
cluster.

**Users/Devops/Developers** is a human user responsible for developing and
Copy link
Contributor

@alebedev87 alebedev87 Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the end user of the Route API is a different actor from the developer which designs the IngressController and Route APIs. The difference between them can be seen, for instance, in the part where the deny list is defined. The user actor cannot influence this list in any form, while the developer has to be define it.
Also what did you mean by the devops actor? How it's different from the user of the API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant application developer

Copy link
Contributor

@alebedev87 alebedev87 Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, then maybe this can be condensed. User/DevOps/Application Developer may simply become User.
Also I think that Developer as an actor representing NetworkEdge Engineer is still needed, for the deny list use case (user story and other mentions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

miheer added a commit to miheer/enhancements that referenced this pull request Jul 27, 2023
It adds changes related to Cookies as to why they are in the deny list.
It mentions the correct override behavior when setting same headers on
Ingress Controller and Route.
It also adds an alternative solution for the override behavior.
miheer added a commit to miheer/enhancements that referenced this pull request Jul 27, 2023
It adds changes related to Cookies as to why they are in the deny list.
It mentions the correct override behavior when setting same headers on
Ingress Controller and Route.
It also adds an alternative solution for the override behavior.
miheer added a commit to miheer/api that referenced this pull request Jul 27, 2023
…y.config Template

This API allows you set fields related to HTTP Response Headers and
Action to be performed i.e Set/Delete via Ingress Controller CR and
Route API.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
`operator/v1/types_ingress.go` - Specifies API to Set/Delete HTTP
Response Headers via Ingress Controller CR
`route/v1/types.go` - Specifies API to Set/Delete HTTP
Response Headers via Route API.
miheer added a commit to miheer/enhancements that referenced this pull request Jul 27, 2023
It adds changes related to Cookies as to why they are in the deny list.
It mentions the correct override behavior when setting same headers on
Ingress Controller and Route.
It also adds an alternative solution for the override behavior.
miheer added a commit to miheer/enhancements that referenced this pull request Jul 30, 2023
It adds changes related to Cookies as to why they are in the deny list.
It mentions the correct override behavior when setting same headers on
Ingress Controller and Route.
It also adds an alternative solution for the override behavior.
miheer added a commit to miheer/api that referenced this pull request Jul 30, 2023
…y.config Template

This API allows you set fields related to HTTP Response Headers and
Action to be performed i.e Set/Delete via Ingress Controller CR and
Route API.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
`operator/v1/types_ingress.go` - Specifies API to Set/Delete HTTP
Response Headers via Ingress Controller CR
`route/v1/types.go` - Specifies API to Set/Delete HTTP
Response Headers via Route API.
miheer added a commit to miheer/api that referenced this pull request Aug 1, 2023
…y.config Template

This API allows you set fields related to HTTP Response Headers and
Action to be performed i.e Set/Delete via Ingress Controller CR and
Route API.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
`operator/v1/types_ingress.go` - Specifies API to Set/Delete HTTP
Response Headers via Ingress Controller CR
`route/v1/types.go` - Specifies API to Set/Delete HTTP
Response Headers via Route API.
miheer added a commit to miheer/api that referenced this pull request Aug 1, 2023
…y.config Template

This API allows you set fields related to HTTP Response Headers and
Action to be performed i.e Set/Delete via Ingress Controller CR and
Route API.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
`operator/v1/types_ingress.go` - Specifies API to Set/Delete HTTP
Response Headers via Ingress Controller CR
`route/v1/types.go` - Specifies API to Set/Delete HTTP
Response Headers via Route API.
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 2, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 2, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 2, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 2, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 3, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 3, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 3, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 4, 2023
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 5, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 6, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 7, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 7, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 7, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 8, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 8, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/openshift-apiserver that referenced this pull request Aug 8, 2023
This also has an unit test to test conversion from versioned to un-versioned
API and vice-versa.
JIRA tiket - https://issues.redhat.com/browse/NE-982
Enhancement Proposal - openshift/enhancements#1296
miheer added a commit to miheer/enhancements that referenced this pull request Aug 20, 2023
It adds changes related to Cookies as to why they are in the deny list.
It mentions the correct override behavior when setting same headers on
Ingress Controller and Route.
It also adds an alternative solution for the override behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants