fix: use staging url for extensions on staging #6140

khendrikse · 2025-03-18T12:41:17Z

🎉 Thanks for submitting a pull request! 🎉

Summary

This ensures that we align our extension API with the right environment.

We use the netlify api host to check if we are on production or staging. This determines the extension api url.

For us to review and ship your PR efficiently, please perform the following steps:

Open a bug/issue before writing your code 🧑‍💻. This ensures
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
Read the contribution guidelines 📖. This ensures
your code follows our style guide and passes our tests.
Update or add tests (if any source code was changed or added) 🧪
Update or add documentation (if features were changed or added) 📝
Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

serhalp · 2025-03-18T13:07:29Z

packages/config/src/api/integrations.ts

 }: GetAvailableIntegrationsOpts): Promise<AvailableIntegration[]> {
  if (offline) {
    return []
  }
  const { host } = testOpts
-  const baseUrl = new URL(host ? `http://${host}/` : `https://api.netlifysdk.com/`)
+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`


nit:

Suggested change

netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`

netlifyApiHost === 'api.netlify.com' ? PRODUCTION_SDK_API_HOST : STAGING_SDK_API_HOST

serhalp · 2025-03-18T13:12:23Z

packages/config/src/api/integrations.ts

 }: GetAvailableIntegrationsOpts): Promise<AvailableIntegration[]> {
  if (offline) {
    return []
  }
  const { host } = testOpts
-  const baseUrl = new URL(host ? `http://${host}/` : `https://api.netlifysdk.com/`)
+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`


I'm not familiar with how this is all set up currently, but this mapping seems a bit brittle.

Would it be a big lift to map all prod/staging/etc. hosts from a single 'mode' or 'env' rather than mapping one set from another?

The assumption that any host that doesn't precisely match api.netlify.com should quietly map to the staging SDK API host seems brittle as well. Could we explicitly map prod -> prod and staging -> staging and throw otherwise? (or at least log an error or something)

100%, I want to try it this way first and then see if I can let buildbot/cli pass in the right url depending on the environment

serhalp · 2025-03-18T13:14:08Z

packages/config/src/api/site_info.ts

-
-  const baseUrl = new URL(host ? `http://${host}` : `https://api.netlifysdk.com`)
+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com' : `https://api-staging.netlifysdk.com `


could we extract these constants somewhere instead of duplicating them in two files? presumably they could just be exported from src/api/integrations.ts

I've moved them to a top level for now

packages/config/src/api/integrations.ts

serhalp · 2025-03-18T13:16:54Z

packages/config/src/api/site_info.ts

+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com' : `https://api-staging.netlifysdk.com `


could we add some basic test coverage for this? It seems pretty critical!

I really want to do that but the test setup is making this quite a struggle right now.

serhalp · 2025-03-18T13:17:31Z

packages/config/src/api/integrations.ts

+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`


could we add some basic test coverage for this? It seems pretty critical!

eduardoboucas · 2025-03-18T13:16:21Z

packages/config/src/api/integrations.ts

-  const baseUrl = new URL(host ? `http://${host}/` : `https://api.netlifysdk.com/`)
+  const extensionApiBaseUrl =
+    netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`
+  const baseUrl = new URL(host ? `http://${host}/` : extensionApiBaseUrl)


Do we ever want to connect to the API over HTTP?

I was wondering the same thing, but I think the assumption here is that the passed in testOpts.host is a local server for testing, so it uses http.

you mean because of the http://${host}/?

packages/config/src/api/site_info.ts

eduardoboucas · 2025-03-18T13:18:53Z

packages/config/src/api/integrations.ts

 }: GetAvailableIntegrationsOpts): Promise<AvailableIntegration[]> {
  if (offline) {
    return []
  }
  const { host } = testOpts
-  const baseUrl = new URL(host ? `http://${host}/` : `https://api.netlifysdk.com/`)
+  const extensionApiBaseUrl =


I think this is fine as a quick fix, but long-term I don't think we should be doing this. If we ever change the API URL, or introduce a variation of it, we'll start to pick the staging SDK endpoint and things will start failing silently in unexpected ways.

We should make it so that whoever passes the API URL also passes the corresponding SDK URL.

:spiderman-pointing:

100% agreed!

I want to try it this way first and then see if I can let buildbot/cli pass in the right url depending on the environment. This truly is a quick fix

packages/config/src/main.ts

fix: use staging url for extensions on staging

8f41be3

khendrikse requested a review from a team as a code owner March 18, 2025 12:41

lemusthelroy previously approved these changes Mar 18, 2025

View reviewed changes

Merge branch 'main' into cpla-2603/use-staging-for-fetching-extensions

43647a0

serhalp previously approved these changes Mar 18, 2025

View reviewed changes

eduardoboucas reviewed Mar 18, 2025

View reviewed changes

khendrikse added 2 commits March 18, 2025 14:26

Merge branch 'main' into cpla-2603/use-staging-for-fetching-extensions

cb0225e

fix: implement some feedback

92e23e0

khendrikse dismissed stale reviews from serhalp and lemusthelroy via 92e23e0 March 18, 2025 13:30

DanielSLew requested changes Mar 18, 2025

View reviewed changes

packages/config/src/main.ts Outdated Show resolved Hide resolved

fix: let production be fallback for extension api

24cb5b6

khendrikse requested a review from DanielSLew March 18, 2025 13:45

DanielSLew approved these changes Mar 18, 2025

View reviewed changes

khendrikse merged commit 978b3c3 into main Mar 18, 2025
31 of 32 checks passed

khendrikse deleted the cpla-2603/use-staging-for-fetching-extensions branch March 18, 2025 14:21

This was referenced Mar 18, 2025

chore: release main #6139

Merged

chore: release main #6142

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use staging url for extensions on staging #6140

fix: use staging url for extensions on staging #6140

khendrikse commented Mar 18, 2025 •

edited

Loading

serhalp Mar 18, 2025

serhalp Mar 18, 2025 •

edited

Loading

khendrikse Mar 18, 2025

serhalp Mar 18, 2025

khendrikse Mar 18, 2025

serhalp Mar 18, 2025

khendrikse Mar 18, 2025

serhalp Mar 18, 2025

eduardoboucas Mar 18, 2025

serhalp Mar 18, 2025

khendrikse Mar 18, 2025

eduardoboucas Mar 18, 2025

serhalp Mar 18, 2025

khendrikse Mar 18, 2025

khendrikse Mar 18, 2025

	netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com/' : `https://api-staging.netlifysdk.com/`
	netlifyApiHost === 'api.netlify.com' ? PRODUCTION_SDK_API_HOST : STAGING_SDK_API_HOST

		const extensionApiBaseUrl =
		netlifyApiHost === 'api.netlify.com' ? 'https://api.netlifysdk.com' : `https://api-staging.netlifysdk.com `

fix: use staging url for extensions on staging #6140

fix: use staging url for extensions on staging #6140

Conversation

khendrikse commented Mar 18, 2025 • edited Loading

Summary

Choose a reason for hiding this comment

serhalp Mar 18, 2025 • edited Loading

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

khendrikse commented Mar 18, 2025 •

edited

Loading

serhalp Mar 18, 2025 •

edited

Loading