refact(refarch-templates)!: modules as dict instead of array#238
refact(refarch-templates)!: modules as dict instead of array#238
Conversation
|
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:
📝 WalkthroughWalkthroughConverts Helm chart modules from a list to a mapping (module keys as names), updates templates to iterate key/value pairs and propagate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
simonhir
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/refarch-templates/templates/deployment.yaml (1)
9-12: Critical:.namereference causes pipeline failure.This is the root cause of the pipeline error
metadata.name 'test-release-%!s(int=0)'. After refactoring to dict-based modules,.nameno longer resolves to the module name—the name is now the dictionary key ($moduleName). The.namereference evaluates to nil or an unexpected type, causing the Go template format error.Proposed fix
{{- if $.Values.imageStream.enabled }} annotations: - {{- include "triggerAnnotation" (list .name $.Values.imageStream.name $.Release.Name ) | trim | nindent 4 }} + {{- include "triggerAnnotation" (list $moduleName $.Values.imageStream.name $.Release.Name ) | trim | nindent 4 }} {{- end }}
🤖 Fix all issues with AI agents
In `@charts/refarch-templates/README.md`:
- Around line 127-131: Add a blank line between the "Example:" line and the
fenced code block that begins with "```yaml" to satisfy MD031; specifically,
update the README's example so the line containing "Example:" is followed by an
empty line before the "```yaml" fence (the fenced code block that contains
"modules:"), ensuring the fence is surrounded by blank lines.
- Around line 123-125: Fix the grammar/typos in the module description
sentences: change "Each module consist of individuell Kubernetes resources (e.g.
Deployment, Service, HPA, ...)." to "Each module consists of individual
Kubernetes resources (e.g. Deployment, Service, HPA, ...)." and ensure
surrounding sentences remain: "Modules consist of individual components in a
dict." and "All configuration options need to be inside a `module`." so replace
"consist"→"consists" and "individuell"→"individual" in the README text.
In `@charts/refarch-templates/templates/deployment.yaml`:
- Around line 2-3: The Helm template uses {{- range $moduleName, $module :=
.Values.modules -}} and {{ $data := dict "dot" $dot "module" $module
"moduleName" $moduleName }} without whitespace trimming; update the template to
add left/right trimming (use {{- and -}}) around both the range and the dict
lines so that the range invocation and the $data definition use trimmed braces
(affecting the range, dict, $data, $moduleName, and $module tokens) to eliminate
YAMLlint brace spacing warnings.
In `@charts/refarch-templates/templates/hpa.yaml`:
- Around line 2-3: Update the Helm template tags to use whitespace trimming so
YAML linting doesn't complain: change the opening and closing action delimiters
around the range and dict lines—specifically the lines using {{- range
$moduleName, $module := .Values.modules -}} and {{ $data := dict "dot" $dot
"module" $module "moduleName" $moduleName }} —to include leading/trailing
hyphens (i.e., use {{- and -}}) so whitespace is trimmed consistently around the
$moduleName, $module and $data declarations.
- Line 17: HPA's scaleTargetRef.name is using {{ $moduleName }} but the
Deployment metadata.name is generated with the helper include "getFullname"
$data, so update the HPA template to reference the same helper; replace the
scaleTargetRef.name value (currently using $moduleName) with the corresponding
include call (the same expression used in the Deployment: include "getFullname"
$data) so both resources share an identical name generation source (ensure you
pass the same $data/context used by the Deployment).
In `@charts/refarch-templates/templates/ingress.yaml`:
- Around line 2-3: The dict assignment for $data is missing whitespace trimming
and leaves extra spaces in rendered YAML; update the directive that defines
$data (the line using $data := dict "dot" $dot "module" $module "moduleName"
$moduleName) to use Helm/Go template whitespace control by changing the
delimiters to {{- ... -}} so it matches the surrounding directives (e.g., the
{{- range $moduleName, $module := .Values.modules -}} block).
In `@charts/refarch-templates/templates/service.yaml`:
- Around line 2-3: The template uses standard Go template delimiters without
whitespace trimming which triggers YAMLlint brace spacing warnings; update the
two template actions by adding trimming markers so the range and dict
invocations use {{- range $moduleName, $module := .Values.modules -}} and {{-
$data := dict "dot" $dot "module" $module "moduleName" $moduleName -}} (i.e.,
replace the opening '{{' with '{{-' and the closing '}}' with '-}}' for the
range and dict expressions in service.yaml) to ensure consistent whitespace
trimming.
In `@charts/refarch-templates/templates/tests/test-connection.yaml`:
- Around line 2-3: The dict assignment for $data lacks whitespace trimming and
should use the same trimmed directive style as the range; update the dict
declaration (the template expression that sets $data with dict "dot" $dot
"module" $module "moduleName" $moduleName) to use {{- and -}} (i.e. change {{
$data := ... }} to a trimmed form) so it matches the existing {{- range ... -}}
usage and satisfies YAMLlint.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/refarch-templates/ci/test-values.yaml (1)
16-27: Update commented backend example to dict format.The commented backend block still uses the old list-based
- namestructure. This is now inconsistent with the dict-basedmoduleslayout and can mislead future edits. Please rewrite it to the newmodules.backend:style (or remove the block).✅ Suggested update
-# - name: backend -# image: -# registry: ghcr.io -# repository: it-at-m/refarch-templates/refarch-backend -# tag: "latest" -# applicationYML: -# spring: -# profiles: -# active: "no-security" -# service: -# http: true +# backend: +# image: +# registry: ghcr.io +# repository: it-at-m/refarch-templates/refarch-backend +# tag: "latest" +# applicationYML: +# spring: +# profiles: +# active: "no-security" +# service: +# http: true
|
@simonhir an sich ein gute Idee, aber ich weiß nicht, ob wir jetzt schon einen Breaking Change veröffentlichen sollten oder warten it-at-m/lhm_actions#106 das wurde auch abgelehnt |
|
@hupling in my opinion as long as correct versioning is used a breaking change is always ok and should be done especially for best practices and in this case the behavior is different than |
|
Meinung von @ejcsid können wir das am Montag besprechen wie vorgehen, z.B.: wir erstellen ein Major-Release für den Change. Brauchen wir 2 Branches (also für 1.x und 2.x)? Pflegen wir Features nur noch in 2.x, machen aber Versions- und Security-Updates auch in 1.x? |
| {{- if $module.env }} | ||
| env: | ||
| {{- toYaml $module.env | nindent 12 }} | ||
| {{- end }} |
| {{- end }} | ||
| {{- if (or $module.volumes $module.applicationYML) }} | ||
| volumes: | ||
| {{- if $module.volumes }} |
There was a problem hiding this comment.
Also allow volumes as map?
3 participants
Description
Summary by CodeRabbit
Chores
Documentation
Refactor