MK8S-197 - UI update to include auth token by ChengYanJin · Pull Request #4827 · scality/metalk8s

ChengYanJin · 2026-03-18T16:29:34Z

No description provided.

bert-e · 2026-03-19T16:16:57Z

Hello chengyanjin,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-03-19T16:17:04Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

bert-e · 2026-03-23T14:52:42Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

shell-ui/src/alerts/AlertProvider.tsx

shell-ui/src/alerts/services/alertManager.ts

ui/src/components/DashboardAlerts.tsx

ui/src/containers/PrometheusAuthProvider.tsx

bert-e · 2026-03-24T13:18:17Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@MonPote

claude · 2026-03-24T16:44:07Z

ui/src/containers/PrometheusAuthProvider.tsx

+
+  useEffect(() => {
+    if (userData?.token) {
+      setHeaders({ Authorization: `Bearer ${userData.token}` });


ui/src/services/alertmanager/api.ts uses ApiClient (which goes through axios), but PrometheusAuthProvider only calls setHeaders on the Prometheus API client, not the AlertManager API client. The AlertManager requests from the UI side will still be unauthenticated. You need to also call setHeaders on the alertmanager API client with the Bearer token, similar to what you did for Prometheus.

— Claude Code

claude · 2026-03-24T16:44:08Z

shell-ui/src/alerts/services/alertManager.ts

 };
-export function getAlerts(alertManagerUrl: string) {
-  return fetch(alertManagerUrl + '/api/v2/alerts')
+export function getAlerts(alertManagerUrl: string, token: string) {


The token parameter is typed as string but it can be undefined (since userData?.token may be undefined). The type should be string | undefined to match the actual usage in AlertProvider.tsx line 26.

— Claude Code

claude · 2026-03-24T16:44:09Z

shell-ui/src/alerts/AlertProvider.tsx

+  const { userData } = useAuth();
+  const token = userData?.token;
+  const query = useQuery(
+    ['activeAlerts', token],


Including the token in the query key means every token refresh/rotation will invalidate the cache and refetch all alerts. Consider using a stable query key (e.g. just activeAlerts) since the token is only needed for the request, not for cache identity.

— Claude Code

claude · 2026-03-24T16:44:10Z

ui/src/containers/PrometheusAuthProvider.tsx

+    }
+  }, [userData?.token]);
+
+  if (!ready) return null;


PrometheusAuthProvider blocks rendering of the entire app (returns null) until the token is available. If userData is slow to resolve or temporarily undefined during token refresh, the entire UI will flash/unmount. Consider rendering children immediately and setting headers as a side effect without blocking, or showing a loading indicator instead of null.

— Claude Code

shell-ui/src/alerts/services/alertManager.ts

claude · 2026-03-24T16:44:31Z

shell-ui/src/alerts/services/alertManager.ts: checkActiveAlertProvider calls getAlerts() with no arguments after the signature was changed to require a token parameter. This will send Authorization: Bearer undefined.
- Pass the token through or make the parameter optional
- shell-ui/src/alerts/services/alertManager.ts: token parameter typed as string but receives undefined from userData?.token.
- Change type to string | undefined (or string?)
- ui/src/containers/PrometheusAuthProvider.tsx: Only sets auth headers on the Prometheus API client, not the AlertManager API client (ui/src/services/alertmanager/api.ts). AlertManager requests from the UI will still be unauthenticated.
- Add a setHeaders export to alertmanager/api.ts and call it alongside the Prometheus one
- shell-ui/src/alerts/AlertProvider.tsx: Token included in the react-query key causes unnecessary cache invalidation on token refresh.
- Use a stable key like ['activeAlerts']
- ui/src/containers/PrometheusAuthProvider.tsx: Returns null until token is available, blocking the entire app tree. May cause UI flash on token refresh.
- Consider rendering children immediately or showing a loader

Review by Claude Code

ui/src/containers/PrometheusAuthProvider.tsx

claude · 2026-03-24T17:47:09Z

ui/src/containers/PrometheusAuthProvider.tsx:20 — setHeaders() is called directly in the render body, causing a side effect on every re-render. This mutates the shared prometheusApiClient during rendering, which violates React's expectation of pure render functions and will trigger warnings in React Strict Mode. Wrap it in useEffect keyed on userData.token.

Review by Claude Code

bert-e · 2026-03-25T10:01:36Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@MonPote

shell-ui/src/alerts/services/alertManager.ts

claude · 2026-03-25T10:04:41Z

shell-ui/src/alerts/services/alertManager.ts: checkActiveAlertProvider calls getAlerts() with no arguments after the signature was changed to require (alertManagerUrl, token). This will fail at runtime.
- Update checkActiveAlertProvider to accept and forward both parameters, or remove it if unused.
ui/src/containers/PrometheusAuthProvider.tsx: Auth headers are only set on the Prometheus API client, not on the AlertManager API client (ui/src/services/alertmanager/api.ts). If the AlertManager client in the ui package is called, those requests will also return 401.
- Add setHeaders to alertmanager/api.ts and call it from this provider, or confirm the client is dead code.

Review by Claude Code

MonPote

LGTM

bert-e · 2026-03-25T12:58:34Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

bert-e · 2026-03-27T10:09:39Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

claude · 2026-03-27T10:16:40Z

ui/src/containers/PrometheusAuthProvider.tsx

+  const { userData } = useAuth();
+  const token = userData?.token;
+
+  useEffect(() => {


Race condition: useEffect runs after render, so when token first becomes available, children render (and can trigger Prometheus API calls) before setHeaders has executed. Use useLayoutEffect instead to ensure headers are set before children mount.

— Claude Code

ui/src/containers/PrometheusAuthProvider.tsx

claude · 2026-03-27T10:16:48Z

Race condition in PrometheusAuthProvider: useEffect fires after render, so children can make Prometheus API calls before setHeaders has been called. Switch to useLayoutEffect to guarantee headers are set before paint/children mount.
AlertManager API client missing auth: The alertmanager client in ui/src/services/alertmanager/api.ts is initialized in the saga but never receives auth headers. Either add setHeaders to the alertmanager API module and call it from the provider, or remove the now-unused getAlerts export to prevent future 401s.

Review by Claude Code

ui/src/containers/PrometheusAuthProvider.tsx

ui/src/services/prometheus/api.ts

claude · 2026-03-27T10:38:01Z

Race condition in PrometheusAuthProvider: useEffect runs child-first in React, so children's react-query hooks may fire Prometheus requests before setHeaders sets the auth token. Use useLayoutEffect instead to guarantee headers are set before child effects run.
- Silent no-op in setHeaders: If prometheusApiClient hasn't been initialized yet, the call is silently ignored and the auth header is permanently lost. Add a warning log or throw.

Review by Claude Code

ui/src/containers/PrometheusAuthProvider.tsx

shell-ui/src/alerts/services/alertManager.ts

ui/src/services/prometheus/api.ts

claude · 2026-03-27T11:16:29Z

PrometheusAuthProvider returns null while token is unavailable, blocking the entire app tree (router, toasts, etc.) from rendering
- Render a loading indicator instead of null, or narrow the provider scope to only Prometheus-dependent components

- useLayoutEffect used in PrometheusAuthProvider for a non-DOM operation (setting API headers)
- Use useEffect instead — no DOM measurement is needed here

- token parameter in getAlerts (alertManager.ts) is optional, allowing unauthenticated requests
- If auth is now required, make the parameter required to catch missing tokens at compile time

- console.warn in prometheus/api.ts setHeaders will fire if called before initialize
- Queue headers for deferred application, or remove the warning since the race is expected

Review by Claude Code

ChengYanJin · 2026-03-27T19:51:43Z

/approve

bert-e · 2026-03-27T19:51:54Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

ChengYanJin changed the title ~~Feature/mk8 s 197 UI update to include auth token~~ MK8S-197 - UI update to include auth token Mar 18, 2026

ChengYanJin force-pushed the feature/MK8S-197-UI-update-to-include-auth-token branch 2 times, most recently from bed75df to fbba288 Compare March 18, 2026 17:05

ChengYanJin changed the title ~~MK8S-197 - UI update to include auth token~~ MK8S-197 - UI prometheus update to include auth token Mar 18, 2026

ChengYanJin force-pushed the feature/MK8S-197-UI-update-to-include-auth-token branch 4 times, most recently from 0b309dd to e001070 Compare March 19, 2026 16:16

ChengYanJin changed the base branch from feature/MK8S-184-restart-script to development/133.0 March 19, 2026 16:16

ChengYanJin marked this pull request as ready for review March 20, 2026 08:41

ChengYanJin requested a review from a team as a code owner March 20, 2026 08:41

ChengYanJin changed the title ~~MK8S-197 - UI prometheus update to include auth token~~ MK8S-197 - UI update to include auth token Mar 20, 2026

ChengYanJin force-pushed the feature/MK8S-197-UI-update-to-include-auth-token branch 4 times, most recently from ce8df24 to 97eeed1 Compare March 24, 2026 09:15

MonPote requested changes Mar 24, 2026

View reviewed changes

claude bot reviewed Mar 24, 2026

View reviewed changes

shell-ui/src/alerts/services/alertManager.ts Outdated Show resolved Hide resolved

ChengYanJin force-pushed the feature/MK8S-197-UI-update-to-include-auth-token branch from fa75385 to d09944d Compare March 24, 2026 17:45

claude bot reviewed Mar 24, 2026

View reviewed changes

ui/src/containers/PrometheusAuthProvider.tsx Outdated Show resolved Hide resolved

ChengYanJin force-pushed the feature/MK8S-197-UI-update-to-include-auth-token branch from d09944d to c15b1d0 Compare March 25, 2026 10:01

claude bot reviewed Mar 25, 2026

View reviewed changes

shell-ui/src/alerts/services/alertManager.ts Outdated Show resolved Hide resolved

MonPote previously approved these changes Mar 25, 2026

View reviewed changes

scality deleted a comment from claude bot Mar 25, 2026

MK8S-197 - Remove checkActiveAlertProvider which is not used anymore

f7ff2bc

ChengYanJin dismissed MonPote’s stale review via f7ff2bc March 27, 2026 10:14

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/containers/PrometheusAuthProvider.tsx Show resolved Hide resolved

MK8S-197 - Remove saga init dead code

63f0757

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/containers/PrometheusAuthProvider.tsx Show resolved Hide resolved

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/services/prometheus/api.ts Show resolved Hide resolved

Claude started work on behalf of ChengYanJin March 27, 2026 10:40 View session

Claude AI changed the title ~~MK8S-197 - UI update to include auth token~~ Addressing PR comments Mar 27, 2026

Claude finished work on behalf of ChengYanJin March 27, 2026 10:41

ChengYanJin changed the title ~~Addressing PR comments~~ MK8S-197 - UI update to include auth token Mar 27, 2026

MK8S-197 - Fix minor issue

8aa0b4d

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/containers/PrometheusAuthProvider.tsx Show resolved Hide resolved

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/containers/PrometheusAuthProvider.tsx Show resolved Hide resolved

claude bot reviewed Mar 27, 2026

View reviewed changes

shell-ui/src/alerts/services/alertManager.ts Show resolved Hide resolved

claude bot reviewed Mar 27, 2026

View reviewed changes

ui/src/services/prometheus/api.ts Show resolved Hide resolved

JBWatenbergScality approved these changes Mar 27, 2026

View reviewed changes

Conversation

ChengYanJin commented Mar 18, 2026 • edited by Claude AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bert-e commented Mar 19, 2026

Hello chengyanjin,

Uh oh!

bert-e commented Mar 19, 2026

Waiting for approval

Uh oh!

bert-e commented Mar 23, 2026

Waiting for approval

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bert-e commented Mar 24, 2026

Waiting for approval

Uh oh!

claude bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude bot commented Mar 24, 2026

Uh oh!

Uh oh!

claude bot commented Mar 24, 2026

Uh oh!

bert-e commented Mar 25, 2026

Waiting for approval

Uh oh!

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

MonPote left a comment

Choose a reason for hiding this comment

Uh oh!

bert-e commented Mar 25, 2026

Waiting for approval

Uh oh!

bert-e commented Mar 27, 2026

Waiting for approval

Uh oh!

claude bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude bot commented Mar 27, 2026

Uh oh!

Uh oh!

Uh oh!

claude bot commented Mar 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude bot commented Mar 27, 2026

Uh oh!

ChengYanJin commented Mar 27, 2026

Uh oh!

bert-e commented Mar 27, 2026

Waiting for approval

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

ChengYanJin commented Mar 18, 2026 •

edited by Claude AI

Loading