chore: runtime env configuration as envFrom#17786
Merged
martinothamar merged 1 commit intomainfrom Feb 18, 2026
Merged
Conversation
github-actions
bot
added
skip-releasenotes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHelm deployment pipeline Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/App/azure-pipelines/deploy-app.yaml (1)
621-625: Consider whetheroptional: false(default) is the intended behaviour for envFrom references.Both
configMapRefandsecretRefdefault tooptional: false, so pods will enterCreateContainerConfigErrorif either resource is missing in the target cluster. This is likely the desired fail-fast behaviour, but it means a missing or mis-named ConfigMap/Secret will block all app deployments in that cluster, not just one.If the ConfigMap and Secret are provisioned by the same chart (v3.10.0) or a shared infrastructure chart, this is fine. Otherwise, consider adding
optional: trueduring the rollout period to avoid a hard dependency on deployment ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/azure-pipelines/deploy-app.yaml` around lines 621 - 625, Review the envFrom entries referencing configMapRef (apps-runtime-common-config-env) and secretRef (apps-runtime-common-secrets-env) and decide if failing the pod when these resources are absent is intended; if not, make the references optional by adding optional: true under each configMapRef/secretRef to avoid CreateContainerConfigError during rollout or deployment-ordering issues, or leave/remove the optional flag if strict fail-fast behavior is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/App/azure-pipelines/deploy-app.yaml`:
- Around line 621-625: Review the envFrom entries referencing configMapRef
(apps-runtime-common-config-env) and secretRef (apps-runtime-common-secrets-env)
and decide if failing the pod when these resources are absent is intended; if
not, make the references optional by adding optional: true under each
configMapRef/secretRef to avoid CreateContainerConfigError during rollout or
deployment-ordering issues, or leave/remove the optional flag if strict
fail-fast behavior is required.
mirkoSekulic
approved these changes
Feb 18, 2026
martinothamar
force-pushed
the
chore/deploy-app-env-from-runtime
branch
from
0454712 to
1b57f18
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Designer/backend/src/Designer/TypedHttpClients/AzureDevOps/Models/QueueBuildRequest.cs`:
- Around line 23-26: The SourceBranch property on class QueueBuildRequest is
seeded with a dev-only default ("chore/deploy-app-env-from-runtime") which
bypasses the JsonIgnore guard; remove the hard-coded default so the property is
null by default (or revert to commented-out dev code) and ensure the
JsonIgnore(Condition = WhenWritingNull) behavior can prevent sending a
sourceBranch unless explicitly set; update the SourceBranch declaration (and
accompanying dev comment) to not provide a non-null default value.
src/Designer/backend/src/Designer/TypedHttpClients/AzureDevOps/Models/QueueBuildRequest.cs
Outdated
Show resolved
Hide resolved
martinothamar
force-pushed
the
chore/deploy-app-env-from-runtime
branch
2 times, most recently
from
961fbff to
2da6ec1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App/azure-pipelines/deploy-app.yaml`:
- Around line 621-625: Add a pipeline pre-flight check to fail fast when
referenced envFrom resources are missing: before pushing artifacts or applying
the HelmRelease, run verification for the ConfigMap and Secret names used in the
manifest (envFrom -> apps-runtime-common-config-env and envFrom ->
apps-runtime-common-secrets-env) for each target cluster/namespace (e.g. kubectl
get configmap apps-runtime-common-config-env and kubectl get secret
apps-runtime-common-secrets-env) and fail the job if either check returns
non-zero; alternatively document and enforce the provisioning order (ensure
infrastructure chart PR `#72` is applied first). Also verify that
apps-runtime-common-config-env contains the environment-specific EFormidling
endpoint keys for tt02/production tiers so the deployed pods get correct
endpoint values.
2da6ec1 to
7b6f93f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/App/azure-pipelines/deploy-app.yaml (1)
582-582: Ensure chart 3.10.0 is published before any deployment triggers this pipeline.If altinn-studio-charts PR
#72has not yet been merged and the chart released to the Helm repository, Flux will not find version3.10.0and every pending HelmRelease will stall inHelmChartNotReady. Co-ordinate the merge/release of the chart PR before or atomically with this pipeline change going live.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/azure-pipelines/deploy-app.yaml` at line 582, The pipeline currently pins the Helm chart with the literal "version: 3.10.0" which will stall Flux if that chart version isn't published; update src/App/azure-pipelines/deploy-app.yaml to either parameterize the chart version (replace the hardcoded version: 3.10.0 with a pipeline variable like $(chartVersion)) or add a pre-deploy validation step that runs "helm repo update" and verifies the chart exists (e.g., "helm search repo <chart-name> --version $(chartVersion)") and fails fast if not found so the deployment does not trigger Flux HelmReleaseNotReady stalls; keep references to the same file and the version token when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App/azure-pipelines/deploy-app.yaml`:
- Around line 621-625: The envFrom block is incorrectly nested under image: and
must be moved into the deployment: section so the Altinn chart will apply
ConfigMap/Secret envs; locate the envFrom keys (envFrom: - configMapRef: name:
apps-runtime-common-config-env - secretRef: name:
apps-runtime-common-secrets-env) and relocate them under the values/deployment:
override for this chart (using the chart's documented deployment.container or
deployment.env syntax) so the templates pick them up; ensure you remove the
envFrom from under image: and follow the chart's expected key names for
environment configuration.
---
Duplicate comments:
In `@src/App/azure-pipelines/deploy-app.yaml`:
- Around line 621-625: The manifest currently assumes existence of
apps-runtime-common-config-env and apps-runtime-common-secrets-env which will
cause CreateContainerConfigError if missing; update the envFrom entries for
configMapRef and secretRef to mark them optional (add optional: true) so pods
start when they’re absent, and/or add a pre-deploy check/Job that creates or
validates the apps-runtime-common-config-env ConfigMap and
apps-runtime-common-secrets-env Secret (or include their manifests in the
release) to ensure the EFormidling endpoint values are present when required.
---
Nitpick comments:
In `@src/App/azure-pipelines/deploy-app.yaml`:
- Line 582: The pipeline currently pins the Helm chart with the literal
"version: 3.10.0" which will stall Flux if that chart version isn't published;
update src/App/azure-pipelines/deploy-app.yaml to either parameterize the chart
version (replace the hardcoded version: 3.10.0 with a pipeline variable like
$(chartVersion)) or add a pre-deploy validation step that runs "helm repo
update" and verifies the chart exists (e.g., "helm search repo <chart-name>
--version $(chartVersion)") and fails fast if not found so the deployment does
not trigger Flux HelmReleaseNotReady stalls; keep references to the same file
and the version token when making the change.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/Designer/backend/src/Designer/TypedHttpClients/AzureDevOps/Models/QueueBuildRequest.cs`:
- Around line 23-26: The SourceBranch property in the QueueBuildRequest class is
left with a hard-coded default ("chore/deploy-app-env-from-runtime") causing
sourceBranch to always be serialized; remove the dev-only default so the
property is null by default (or revert the property to its commented-out state)
and keep the JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)
decorator so sourceBranch is omitted from serialization unless explicitly set;
update the SourceBranch declaration accordingly and ensure no other code relies
on the hard-coded string.
ce7c7d1 to
7b6f93f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
depends on Altinn/altinn-studio-charts#72
secrets and app-specific stuff must be handled separately
Verification
Summary by CodeRabbit