Skip to content

frontend: Container env vars: show secret value for secretKeyRef #4706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pavitar-rana wants to merge 1 commit into
base: main
Choose a base branch
from
+67 −109
Open

frontend: Container env vars: show secret value for secretKeyRef #4706

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 67 additions & 109 deletions frontend/src/components/common/Resource/Resource.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import Paper from '@mui/material/Paper';
import { BaseTextFieldProps } from '@mui/material/TextField';
import Typography from '@mui/material/Typography';
import { useTheme } from '@mui/system';
import { useQueries } from '@tanstack/react-query';
import { Location } from 'history';
import { Base64 } from 'js-base64';
import { JSONPath } from 'jsonpath-plus';
Expand All @@ -38,6 +39,10 @@ import { generatePath, NavLinkProps, useLocation } from 'react-router-dom';
import YAML from 'yaml';
import { labelSelectorToQuery, ResourceClasses, useCluster } from '../../../lib/k8s';
import { ApiError } from '../../../lib/k8s/api/v2/ApiError';
import { clusterFetch } from '../../../lib/k8s/api/v2/fetch';
import { useEndpoints } from '../../../lib/k8s/api/v2/hooks';
import { KubeObjectEndpoint } from '../../../lib/k8s/api/v2/KubeObjectEndpoint';
import { makeUrl } from '../../../lib/k8s/api/v2/makeUrl';
import { KubeCondition, KubeContainer, KubeContainerStatus } from '../../../lib/k8s/cluster';
import ConfigMap from '../../../lib/k8s/configMap';
import { KubeEvent } from '../../../lib/k8s/event';
Expand Down Expand Up @@ -751,46 +756,45 @@ function extractEnvVarReferences(container: KubeContainer): EnvVarReference[] {
}

/**
* Component that fetches a Secret and reports the result.
* This properly calls hooks at the top level.
* Fetches a list of KubeObjects by name using useQueries (fetch-only, no WebSocket watches).
* Returns a Map<name, FetchedResource> compatible with buildEnvironmentVariables.
*/
function SecretFetcher(props: {
name: string;
namespace: string;
onResult: (name: string, resource: KubeObject | null, error: ApiError | null) => void;
}) {
const { name, namespace, onResult } = props;
const [secret, error] = Secret.useGet(name, namespace);

React.useEffect(() => {
// Only call onResult when we have a definitive result (either data or error)
if (secret || error) {
onResult(name, secret, error);
}
}, [secret, error, name, onResult]);

return null;
}

/**
* Component that fetches a ConfigMap and reports the result.
* This properly calls hooks at the top level.
*/
function ConfigMapFetcher(props: {
name: string;
namespace: string;
onResult: (name: string, resource: KubeObject | null, error: ApiError | null) => void;
}) {
const { name, namespace, onResult } = props;
const [configMap, error] = ConfigMap.useGet(name, namespace);

React.useEffect(() => {
if (configMap || error) {
onResult(name, configMap, error);
}
}, [configMap, error, name, onResult]);
function useResourcesFetchOnly(
kubeObjectClass: typeof Secret | typeof ConfigMap,
names: string[],
namespace: string,
cluster: string
): Map<string, FetchedResource> {
const { endpoint } = useEndpoints(kubeObjectClass.apiEndpoint.apiInfo, cluster);

const results = useQueries({
queries: names.map(name => ({
queryKey: ['env-resource', cluster, namespace, kubeObjectClass.apiName, name],
enabled: !!endpoint,
staleTime: 30_000,
queryFn: async (): Promise<FetchedResource> => {
try {
const url = makeUrl([KubeObjectEndpoint.toUrl(endpoint!, namespace), name]);
const obj = await clusterFetch(url, { cluster }).then(r => r.json());
return { resource: new kubeObjectClass(obj) as KubeObject, error: null };
} catch (err) {
return { resource: null, error: err as ApiError };
}
},
})),
});

return null;
return React.useMemo(() => {
const map = new Map<string, FetchedResource>();
names.forEach((name, idx) => {
const result = results[idx];
if (result?.data) {
map.set(name, result.data);
}
});
return map;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [names, ...results.map(r => r.data)]);
Comment on lines +796 to +797
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency array contains a spread of results.map(r => r.data), which creates a new array on every render. This causes the useMemo to recompute on every render, defeating its purpose. Consider using a different approach such as:

  1. Use the combine option in useQueries to transform the results directly (similar to the pattern in useKubeObjectList.ts)
  2. Or, track individual result data items by adding each result.data to the dependency array without spreading, though this is verbose
  3. Or, use a custom deep comparison function

The current implementation with the eslint-disable comment suggests this was an intentional workaround, but it's worth reconsidering for performance.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@illume illume Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems valid.

}

/**
Expand Down Expand Up @@ -1027,36 +1031,10 @@ export function ContainerEnvironmentVariables(props: EnvironmentVariablesProps)
const { pod, container } = props;
const { t } = useTranslation();
const { enqueueSnackbar } = useSnackbar();

// State to store fetched resources
const [fetchedSecrets, setFetchedSecrets] = React.useState<Map<string, FetchedResource>>(
new Map()
);
const [fetchedConfigMaps, setFetchedConfigMaps] = React.useState<Map<string, FetchedResource>>(
new Map()
);

// Early return if no env vars
if (
(!container?.env && !container?.envFrom) ||
!pod?.status?.containerStatuses ||
!pod?.metadata?.namespace
) {
return null;
}

const namespace = pod.metadata.namespace;
const containerStartTimestamp = (() => {
let timestamp = pod.metadata?.creationTimestamp;
const containerStatus = pod.status?.containerStatuses?.find(c => c.name === container?.name);
if (containerStatus?.started && containerStatus.state?.running?.startedAt) {
timestamp = containerStatus.state.running.startedAt;
}
return timestamp;
})();
const cluster = useCluster() ?? '';

// Extract all references upfront (pure function, no hooks)
const references = extractEnvVarReferences(container);
const references = extractEnvVarReferences(container as KubeContainer);
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type cast container as KubeContainer is unnecessary and potentially misleading. The function extractEnvVarReferences already uses optional chaining (container?.env, container?.envFrom) to handle undefined containers safely. The type declaration for container in EnvironmentVariablesProps already indicates it can be undefined (container?: KubeContainer), so the cast doesn't add type safety and could hide issues.

Consider removing the type cast and letting TypeScript use the natural type.

Suggested change
const references = extractEnvVarReferences(container as KubeContainer);
const references = extractEnvVarReferences(container);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@illume illume Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth a look. Usually it is a good idea to avoid using as because generally it hides bugs. Sometimes you need to handle the cases it brings up.

In rare cases it can be there’s a reason the types don’t work or the types are wrong and as can work around that. If this is one of those cases please add a comment there explaining the issue. Then someone later can get to it.


// Get unique resource names to fetch
const secretsToFetch = React.useMemo(() => {
Expand All @@ -1079,28 +1057,29 @@ export function ContainerEnvironmentVariables(props: EnvironmentVariablesProps)
return Array.from(configMaps);
}, [references]);

// Callbacks to handle fetched resources
const handleSecretFetched = React.useCallback(
(name: string, resource: KubeObject | null, error: ApiError | null) => {
setFetchedSecrets(prev => {
const next = new Map(prev);
next.set(name, { resource, error });
return next;
});
},
[]
);
const namespace = pod?.metadata?.namespace ?? '';

const handleConfigMapFetched = React.useCallback(
(name: string, resource: KubeObject | null, error: ApiError | null) => {
setFetchedConfigMaps(prev => {
const next = new Map(prev);
next.set(name, { resource, error });
return next;
});
},
[]
);
// Fetch secrets and configmaps in parallel — no WebSocket watchers
const fetchedSecrets = useResourcesFetchOnly(Secret, secretsToFetch, namespace, cluster);
const fetchedConfigMaps = useResourcesFetchOnly(ConfigMap, configMapsToFetch, namespace, cluster);

// Early return if no env vars
if (
(!container?.env && !container?.envFrom) ||
!pod?.status?.containerStatuses ||
!pod?.metadata?.namespace
) {
return null;
}

const containerStartTimestamp = (() => {
let timestamp = pod.metadata?.creationTimestamp;
const containerStatus = pod.status?.containerStatuses?.find(c => c.name === container?.name);
if (containerStatus?.started && containerStatus.state?.running?.startedAt) {
timestamp = containerStatus.state.running.startedAt;
}
return timestamp;
})();

// Copy handler using notistack
const handleCopy = React.useCallback(
Expand Down Expand Up @@ -1229,28 +1208,7 @@ export function ContainerEnvironmentVariables(props: EnvironmentVariablesProps)
},
];

return (
<>
{/* Render fetcher components - these call hooks properly at top level */}
{secretsToFetch.map(name => (
<SecretFetcher
key={`secret-${name}`}
name={name}
namespace={namespace}
onResult={handleSecretFetched}
/>
))}
{configMapsToFetch.map(name => (
<ConfigMapFetcher
key={`configmap-${name}`}
name={name}
namespace={namespace}
onResult={handleConfigMapFetched}
/>
))}
<InnerTable columns={columns} data={variables} />
</>
);
return <InnerTable columns={columns} data={variables} />;
}

export interface VolumeMountsProps {
Expand Down
Loading