[Banner Plugin] Dynamic Configuration Support for the Banner Plugin #10326

sidwang42 · 2025-08-03T21:34:52Z

Description

This PR introduces dynamic configuration support for the banner plugin.
A new optional externalLink setting allows OpenSearch Dashboards to fetch banner configuration
from a remote JSON endpoint. If fetching fails, the system falls back to local YAML or Advanced Settings.
This enables centralized banner management across multiple instances.

Issues Resolved

#9990

Screenshot

No direct UI changes, but banner content, style, and visibility can now
be updated dynamically from an external configuration endpoint.

Testing the changes

Add the following entry to opensearch_dashboards.yml:
banner.externalLink: "https://example.com/banner-config.json"
Start OpenSearch Dashboards.
Verify the banner loads configuration from the provided endpoint.
Simulate a failed request (e.g., invalid URL) — confirm fallback to local configuration.
Run unit tests in fetch_external_config.test.ts:
yarn test:jest src/plugins/banner/server/routes/fetch_external_config.test.ts
Validate both HTTP and HTTPS URLs work, including timeout and parsing error scenarios.

Changelog

feat: Support dynamic banner config via external JSON endpoint with local fallback

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update [CHANGELOG.md](./../CHANGELOG.md)
Commits are signed per the DCO using --signoff

Signed-off-by: Sid Wang <[email protected]>

codecov · 2025-08-04T17:29:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.84%. Comparing base (b886e61) to head (173c445).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10326      +/-   ##
==========================================
+ Coverage   61.76%   61.84%   +0.08%     
==========================================
  Files        4242     4255      +13     
  Lines      107874   108350     +476     
  Branches    17572    17725     +153     
==========================================
+ Hits        66623    67012     +389     
- Misses      36775    36827      +52     
- Partials     4476     4511      +35

Flag	Coverage Δ
Linux_1	`27.64% <ø> (-0.01%)`	⬇️
Linux_2	`41.23% <ø> (ø)`
Linux_3	`39.60% <100.00%> (+0.06%)`	⬆️
Linux_4	`33.01% <ø> (+0.24%)`	⬆️
Windows_1	`27.66% <ø> (-0.01%)`	⬇️
Windows_2	`41.20% <ø> (ø)`
Windows_3	`39.60% <100.00%> (+0.06%)`	⬆️
Windows_4	`33.01% <ø> (+0.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

zhongnansu · 2025-08-08T22:23:11Z

src/plugins/banner/server/routes/fetch_external_config.ts

+        },
+        (res) => {
+          if (res.statusCode !== 200) {
+            logger.error(`Error fetching banner config: HTTP status ${res.statusCode}`);


let's also log the error message for troubleshooting

zhongnansu · 2025-08-08T22:25:48Z

src/plugins/banner/server/routes/fetch_external_config.ts

+  return new Promise((resolve) => {
+    try {
+      const parsedUrl = new URL(url);
+      const requestModule = parsedUrl.protocol === 'https:' ? https : http;


we should force https for security

zhongnansu · 2025-08-08T22:28:51Z

src/plugins/banner/server/routes/fetch_external_config.ts

+      const parsedUrl = new URL(url);
+      const requestModule = parsedUrl.protocol === 'https:' ? https : http;
+
+      const req = requestModule.request(


Can we use fetch() in Node 18+ (or node-fetch), which handles timeouts, errors, and streaming internally?

zhongnansu · 2025-08-08T22:29:54Z

src/plugins/banner/server/routes/fetch_external_config.ts

+
+          let data = '';
+          res.on('data', (chunk) => {
+            data += chunk;


where do we check the json endpoint format is matching to what we required?

where do we check the json endpoint format is matching to what we required?

@zhongnansu
Haven’t added this yet, as I was assuming all content would come from a system-admin-set value, so no check was needed.

But thinking about potential issues like malformed JSON, missing required fields, or incorrect data types causing runtime errors or unexpected UI behavior, it would be great to add a validation check here.

system-admin-set values set by human, we don't assume things are working as expected, we prevent things from going south.

Signed-off-by: Sid Wang <[email protected]>

zhongnansu · 2025-08-12T17:38:40Z

src/plugins/banner/server/config.ts

@@ -28,4 +28,5 @@ export const configSchema = schema.object({
  size: schema.oneOf([schema.literal('s'), schema.literal('m')], {
    defaultValue: 'm',
  }),
+  externalLink: schema.maybe(schema.string()),


is there a schema for url? If not, can we add url validation?

zhongnansu · 2025-08-12T17:39:05Z

src/plugins/banner/server/validate_banner_config.ts

+  // Check externalLink
+  if (config.externalLink !== undefined && typeof config.externalLink !== 'string') {
+    logger.error(
+      `Invalid banner config: 'externalLink' must be a string, got ${typeof config.externalLink}`


not just a valid string, it should be a valid url

zhongnansu · 2025-08-12T17:44:06Z

src/plugins/banner/server/routes/fetch_external_config.ts

+ * @param logger Logger instance for logging errors
+ * @returns The banner configuration or null if there was an error
+ */
+export async function fetchExternalConfig(


this function does not only fetch, but also validate. Consider extract the validation logic, and make 2 function calls, one to fetch, one for validate content

zhongnansu · 2025-08-12T17:51:03Z

src/plugins/banner/server/validate_banner_config.ts

+ * @param logger Logger instance for logging errors
+ * @returns Whether the configuration is valid
+ */
+export function validateBannerConfig(


How do we ensure that invalid fields are not passed in?
For example

{ content: "validstring" somethingBad: "some bad scripts that should not be passed in" }

zhongnansu

codecov CI is failing, could you check if we need more tests?

Signed-off-by: Sid Wang <[email protected]>

zhongnansu

lgtm

huyaboo · 2025-08-13T21:47:46Z

src/plugins/banner/server/routes/get_config.ts

+          if (externalConfig) {
+            // Validate the configuration
+            if (!validateBannerConfig(externalConfig, bannerSetup.logger)) {


instead of nesting these if statements, can we invert the logic so it's more readable? Eg:

if (!externalConfig) { // Failed to fetch from URL // return response } if (!validateBannerConfig) { // invalid config // return response } // return with externalconfig

instead of nesting these if statements, can we invert the logic so it's more readable? Eg:

if (!externalConfig) { // Failed to fetch from URL // return response } if (!validateBannerConfig) { // invalid config // return response } // return with externalconfig

Great Point. Updated accordingly. 😄

huyaboo · 2025-08-13T21:51:24Z

src/plugins/banner/server/validate_banner_config.ts

+  let isValid = true;
+
+  // Check for unexpected fields
+  const validFields = [


Correct me if I'm wrong but we defined a schema somewhere within the banner plugin codebase right? Could we not use that schema to validate?

@huyaboo

Correct me if I'm wrong but we defined a schema somewhere within the banner plugin codebase right? Could we not use that schema to validate?

I aim for the same — having a centralized control of the allowed schema — but it doesn’t seem possible in this case. The configSchema in config.ts is only applied by the platform during YAML load at plugin startup. For dynamic configs (e.g., loaded from link or other sources at runtime), there’s no built-in hook to reuse that schema object directly for validation without re-implementing it, because @osd/config-schema is tightly coupled to the initial config lifecycle rather than arbitrary runtime input.

Signed-off-by: Sid Wang <[email protected]>

[Banner Plugin] Basic for dynamic config is done

070bb16

Signed-off-by: Sid Wang <[email protected]>

github-actions bot added the repeat-contributor label Aug 3, 2025

opensearch-changeset-bot bot added a commit to sidwang42/OpenSearch-Dashboards that referenced this pull request Aug 3, 2025

Changeset file for PR opensearch-project#10326 created/updated

a22f824

Changeset file for PR opensearch-project#10326 created/updated

ef701d1

sidwang42 force-pushed the banner_dynamic_config branch from a22f824 to ef701d1 Compare August 3, 2025 22:20

sidwang42 marked this pull request as ready for review August 4, 2025 02:21

sidwang42 requested review from angle943, Maosaic, wanglam, d-buckner, raintygao and ps48 as code owners August 4, 2025 02:21

Merge branch 'main' into banner_dynamic_config

910358c

Merge branch 'main' into banner_dynamic_config

6ba7134

sidwang42 mentioned this pull request Aug 3, 2025

[Meta][Banner Plugin] Feature, bug fix and enhancement for Banner Plugin #9990

Open

16 tasks

zhongnansu added the banner Dashboards banner plugin which displays global banner and allows customization label Aug 5, 2025

Merge branch 'opensearch-project:main' into banner_dynamic_config

478fb3e

zhongnansu reviewed Aug 8, 2025

View reviewed changes

sidwang42 added 4 commits August 11, 2025 10:02

Merge branch 'main' into banner_dynamic_config

e7ec62b

[Banner Plugin] Using fetch instead and adding validate banner config

592af31

Signed-off-by: Sid Wang <[email protected]>

Merge branch 'main' into banner_dynamic_config

d15d459

[Banner Plugin] Update unit tests

28913d8

Signed-off-by: Sid Wang <[email protected]>

zhongnansu reviewed Aug 12, 2025

View reviewed changes

sidwang42 added 4 commits August 12, 2025 11:52

[Banner Plugin] Add url validation, field validation, and unit tests

b1dfb38

Signed-off-by: Sid Wang <[email protected]>

[Banner Plugin] Update Schema

d82ae6f

Signed-off-by: Sid Wang <[email protected]>

Merge branch 'main' into banner_dynamic_config

ba70d24

[Banner Plugin] Add unit test for validate banner

56f0e83

Signed-off-by: Sid Wang <[email protected]>

zhongnansu previously approved these changes Aug 12, 2025

View reviewed changes

huyaboo reviewed Aug 13, 2025

View reviewed changes

[Banner Plugin] Update the if else logic

173c445

Signed-off-by: Sid Wang <[email protected]>

sidwang42 dismissed zhongnansu’s stale review via 173c445 August 15, 2025 18:01

zhongnansu approved these changes Aug 16, 2025

View reviewed changes

zhongnansu requested a review from huyaboo August 16, 2025 05:02

[Banner Plugin] Dynamic Configuration Support for the Banner Plugin #10326

Are you sure you want to change the base?

[Banner Plugin] Dynamic Configuration Support for the Banner Plugin #10326

Uh oh!

Conversation

sidwang42 commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

Uh oh!

codecov bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhongnansu left a comment

Choose a reason for hiding this comment

Uh oh!

zhongnansu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sidwang42 commented Aug 3, 2025 •

edited

Loading

codecov bot commented Aug 4, 2025 •

edited

Loading