task: Add customHeaders as possible parameter. #4139

chriswk · 2023-07-04T13:56:42Z

###What
Adds an optional sensitive parameter for customHeaders to all current addons. When formatted as a valid json object, the object will be included as headers in the request the addon makes upstream to it's target URL

Adds support for custom headers for all current addons.

vercel · 2023-07-04T13:56:48Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
unleash-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 5, 2023 7:04am
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 5, 2023 7:04am

gastonfournier · 2023-07-04T14:28:55Z

src/lib/addons/webhook.test.ts

+        url: 'http://test.webhook.com/plain',
+        bodyTemplate: '{{event.type}} on toggle {{event.data.name}}',
+        contentType: 'text/plain',
+        authorization: 'API KEY 123abc',


I think it can safely be dismissed

Indeed, since it's a test, but it's good to see CodeQL catching these, just in case we stupidly add them to production code. :)

gastonfournier

I think this will work, just maybe a few improvements

gastonfournier · 2023-07-04T14:29:49Z

src/lib/addons/__snapshots__/slack.test.ts.snap

@@ -1,9 +1,18 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`Should call slack webhook 1`] = `"{"username":"Unleash","icon_emoji":":unleash:","text":"some@user.com created feature toggle <http://some-url.com/projects/default/features/some-toggle|some-toggle> in project *default*","channel":"#undefined","attachments":[{"actions":[{"name":"featureToggle","text":"Open in Unleash","type":"button","value":"featureToggle","style":"primary","url":"http://some-url.com/projects/default/features/some-toggle"}]}]}"`;
+exports[`Should call slack webhook 1`] = `"{"username":"Unleash","icon_emoji":":unleash:","text":"some@user.com created feature toggle <http://some-url.com/projects/default/features/some-toggle|some-toggle> in project *default*","channel":"#general","attachments":[{"actions":[{"name":"featureToggle","text":"Open in Unleash","type":"button","value":"featureToggle","style":"primary","url":"http://some-url.com/projects/default/features/some-toggle"}]}]}"`;


Have you fixed slack integration by chance?

No, have not changed it, just given it support for custom headers

It changed from undefined to general so the support for custom headers might have helped. Not sure but 👍

Oh, that's just because I'm now type checking the parameters, so our test no longer allowed us to activate the slack addon with no defined defaultChannel (it's marked as required in our definition). Earlier, the test that wrote this snapshot did not have the defaultChannel parameter set, which caused javascript to happily send undefined :)

gastonfournier · 2023-07-04T14:40:11Z

src/lib/addons/datadog.ts

+interface IDatadogParameters {
+    url: string;
+    apiKey: string;
+    customHeaders?: string;


Why not something like:

customHeaders?: { [key: string]: string } | Record<string, string> ;

That would save you from doing JSON.parse later

Inspired by https://github.com/search?q=%22type+HeadersInit+%3D%22+language%3ATypeScript&type=code&l=TypeScript

Hmm, if it works that would be good, we just don't have a way to define the input parameter in the addon definition as anything but a string. So somewhere we'd need to do the parsing.

gastonfournier · 2023-07-04T14:43:24Z

src/lib/addons/datadog.ts

+                extraHeaders = JSON.parse(customHeaders);
+            } catch (e) {
+                this.logger.warn(
+                    'Could not parse the json in the customHeaders parameters',


Suggested change

'Could not parse the json in the customHeaders parameters',

`Could not parse the json in the customHeaders parameters ${customHeaders}`,

but I think this exception should bubble up as it'd be a configuration issue

But you wouldn't want to throw an exception when creating a feature because one of the addons failed to parse data.

Though, this is on a different thread.

gastonfournier · 2023-07-04T14:45:53Z

src/lib/addons/slack.ts

+    username?: string;
+    defaultChannel: string;
+    emojiIcon?: string;
+    customHeaders?: string;


Maybe this would benefit from the same suggestion:

customHeaders?: { [key: string]: string } | Record<string, string> ;

Same comment here, since we're doing a post of parameters, it isn't currently possible to have it as anything but strings, otherwise our template for event data would also be something else but string. I tested locally to ensure that this was correct.

gastonfournier · 2023-07-04T14:46:09Z

src/lib/addons/teams.ts

@@ -8,6 +8,10 @@ import {
 } from './feature-event-formatter-md';
 import { IEvent } from '../types/events';

+interface ITeamsParameters {
+    url: string;
+    customHeaders?: string;


gastonfournier · 2023-07-04T14:47:20Z

src/lib/addons/webhook.ts

@@ -9,6 +9,7 @@ interface IParameters {
    bodyTemplate?: string;
    contentType?: string;
    authorization?: string;
+    customHeaders?: string;


And one more... although this could be a bit trickier because we need to handle a web request.

gastonfournier · 2023-07-04T14:48:25Z

src/lib/types/model.ts

+export interface ICustomHeaders {
+    [name: string]: string;
+}
+


Ok, I think you had the idea of doing something as I suggested, but I believe this is not used right now

You're right. I forgot to remove this one.

gastonfournier

LG!

Adds an optional sensitive parameter for customHeaders to all current addons. It is sensitive because the user might be including api key headers.

## About the changes Early release of #4139 and #4196 as 5.2 patch --------- Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>

task: Add customHeaders as possible parameter.

854221d

Adds support for custom headers for all current addons.

chriswk requested review from gastonfournier, nunogois and sighphyre July 4, 2023 13:56

chriswk self-assigned this Jul 4, 2023

vercel bot deployed to Preview – unleash-monorepo-frontend July 4, 2023 13:57 View deployment

github-advanced-security bot found potential problems Jul 4, 2023

View reviewed changes

gastonfournier reviewed Jul 4, 2023

View reviewed changes

chriswk added 2 commits July 5, 2023 08:56

Remove unused type

e5a66e4

Output customHeaders that we failed to parse to log

cfe9915

vercel bot deployed to Preview – unleash-monorepo-frontend July 5, 2023 06:59 View deployment

vercel bot deployed to Preview – unleash-docs July 5, 2023 07:04 View deployment

gastonfournier approved these changes Jul 5, 2023

View reviewed changes

chriswk merged commit 186fda1 into main Jul 5, 2023
12 of 17 checks passed

chriswk deleted the task/customHeadersAddons branch July 5, 2023 07:42

gastonfournier pushed a commit that referenced this pull request Jul 12, 2023

task: Add customHeaders as possible parameter. (#4139)

c9a3ff6

Adds an optional sensitive parameter for customHeaders to all current addons. It is sensitive because the user might be including api key headers.

gastonfournier mentioned this pull request Jul 12, 2023

chore: prepare 5.2 patch #4224

Merged

gastonfournier added a commit that referenced this pull request Jul 12, 2023

chore: prepare 5.2 patch (#4224)

1bed7fc

## About the changes Early release of #4139 and #4196 as 5.2 patch --------- Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

task: Add customHeaders as possible parameter. #4139

task: Add customHeaders as possible parameter. #4139

chriswk commented Jul 4, 2023

vercel bot commented Jul 4, 2023 •

edited

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

gastonfournier left a comment

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

gastonfournier Jul 5, 2023

chriswk Jul 5, 2023

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

chriswk Jul 5, 2023

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

gastonfournier Jul 4, 2023

gastonfournier Jul 4, 2023

gastonfournier Jul 4, 2023

chriswk Jul 5, 2023

gastonfournier left a comment

	'Could not parse the json in the customHeaders parameters',
	`Could not parse the json in the customHeaders parameters ${customHeaders}`,

task: Add customHeaders as possible parameter. #4139

task: Add customHeaders as possible parameter. #4139

Conversation

chriswk commented Jul 4, 2023

vercel bot commented Jul 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gastonfournier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gastonfournier left a comment

Choose a reason for hiding this comment

vercel bot commented Jul 4, 2023 •

edited