feat(http source): Add custom response header configuration

[ChrisCanCompute]

trafficstars

Add a custom_response_headers option to SimpleHttpConfig to add the given headers to all Http responses.

Closes: #20395

[bits-bot]

All committers have signed the CLA.

[ChrisCanCompute]

Hi, I have seen your comment. I've been having a hard time running make generate-component-docs, but I have got a test. I'm unlikely to get any changes before next week, but hopefully should get the rest of this up soon after that.

Thanks for the quick review!

[jszwedko]

Hi, I have seen your comment. I've been having a hard time running make generate-component-docs, but I have got a test. I'm unlikely to get any changes before next week, but hopefully should get the rest of this up soon after that.

Thanks for the quick review!

Ah, I think this is actually failing to compile at the moment, which is why make generate-component-docs is failing 😄

[ChrisCanCompute]

The build should be fixed now. I'll sort out docs and a test soon.

[ChrisCanCompute]

The code should all now be good. I have:

• An old machine connected to the internet.
• A new machine that's air gapped.

I haven't been able to get either of them to generate the docs. I can try and do it manually, but it would be much quicker if someone else was able to run the relevant command on my branch. Otherwise I can try and write them by hand?

I'll sort out the commits that are missing author emails (not sure how that happened).

[jszwedko]

Thanks @ChrisCanCompute . It compiles now, but I'm getting this error when generating the docs:

For map fields (`HashMap<...>`, etc), a description (in the singular form) must be provided by via `#[configurable(metadata(docs::additional_props_description = "Description of the field."))]`.

The description on the field, derived from the code comments, is shown specifically for `field`, while the description provided via `docs::additional_props_description` is shown for the special `field.*` entry that denotes that the field is actually a map.
[2024-07-29T15:34:13] ERROR Relevant schema: {
  "description": "Custom response headers to be added to the HTTP response",
  "default": {
  },
  "type": "object",
  "additionalProperties": {
    "type": "string"
  },
  "_metadata": {
    "docs::examples": {
      "Access-Control-Allow-Origin": "my-cool-server"
    },
    "docs::human_name": "Custom Response Headers"
  }
}
Error: command: cd "/Users/jesse.szwedko/workspace/vector" && "/Users/jesse.szwedko/workspace/vector/scripts/generate-component-docs.rb" "/tmp/vector-config-schema.json"
  failed with exit code: 1
make: *** [generate-component-docs] Error 1

I think we need to add the field description as suggested there.

[ChrisCanCompute]

Got the docs updated as well. Just need to fix the branch to have an email on the two commits that are missing one.

[ChrisCanCompute]

I've done a force push to my branch to fix the authors (so that the CLA check passes). There's no code diff since your approval.

Let me know if there's anything else you need from me on this PR.

[jszwedko]

@ChrisCanCompute Apologies, can you merge in master here to fix CI? I tried to, but don't have write access to this branch.

[xiazuojie]

HTTP allows set multiple headers with the same name, e.g. multiple set-cookie headers. So the data structure for custom_response_headers would better be HashMap<String, List<String>>

[jszwedko]

HTTP allows set multiple headers with the same name, e.g. multiple set-cookie headers. So the data structure for custom_response_headers would better be HashMap<String, List<String>>

Good point. I'm curious what you think of changing this to take a map of header name to list of header values @ChrisCanCompute ? Ideally it could handle either one value or a list of values since single value for each header is more common but that could be more difficult so I'd be happy to see it take a list of values.

[ChrisCanCompute]

I've updated this to take multiple strings. I initially looped through each value and called warp::reply::with_header(key, value), but the test showed that the values were being overwritten (which surprised me a bit). So I've gone for .join(", ") instead.

Let me know what you think, and thank you for the feedback so far!

[jorgehermo9]

I think it would be better to use https://docs.rs/http/latest/http/header/struct.HeaderMap.html#method.try_append and get the HeaderMap through https://docs.rs/warp/latest/warp/reply/type.Response.html#method.headers

Insert the first value, and then append the rest

@ChrisCanCompute

[ChrisCanCompute]

@jorgehermo9 I've updated the PR to use append and insert. We're on http 0.2.9, so don't have try_append and try_insert and I didn't fancy doing a major version upgrade of the http dependency in this PR.

[jorgehermo9]

I think it would be a possible panic that the user specifies a lot of headers (for any reason) in the config and then the append or `insert’ calls panics instead of giving a proper error.

Maybe should we create an issue stating this possible enhancement so it is tracked?

[ChrisCanCompute]

@jszwedko thanks for the feedback. I've addressed it and this is hopefully good to go now!

[pront]

Note: A few CI checks are failing.