Skip to content

feat: support templating in diagnose command#9393

Merged
ericzzzzzzz merged 1 commit into
GoogleContainerTools:mainfrom
ericzzzzzzz:support-templating
May 3, 2024
Merged

feat: support templating in diagnose command#9393
ericzzzzzzz merged 1 commit into
GoogleContainerTools:mainfrom
ericzzzzzzz:support-templating

Conversation

@ericzzzzzzz

@ericzzzzzzz ericzzzzzzz commented Apr 19, 2024

Copy link
Copy Markdown
Contributor

Fixes: #8758

Description

  • add support templating in diagnose command when using --yaml-only
    • add skaffold:template tag to label templated fields, so we can use go template engine to render the specified fields, supported fields are the same as other commands see templated fields ,
    • the feature is guarded by a flag enable-templating in diagnose command, as this feature is a breaking change for users are keeping template place holder when using diagnose command.
    • The implementation doesn't have special handling for templated fields for ko builder as other command which replace {{.ENV with {{, that was for ko-skaffold templates compatibilities , but we've documented that for more than one year https://skaffold.dev/docs/builders/builder-types/ko/#build-flags .ENV is not needed, so treating it as normal templated fields should be fine.
    • The implementation will error out if key is missing when rendering templates.

Schema change

Follow-up Work

  • the skaffold:template may be used for generating docs to show which fields are templated fields, we may add an issue if the approach in this pr is the way we want to implement the feature .

Alternative Solution

  • unmarshal the whole config and then render with go template.
    • Pros:
      • easy to implement
    • Cons:
      • The behavior is not consistent with other commands and the whole skaffold.yaml might be taken as a template file, misuse can lead to unexpected result when skaffold marshal skaffold.yaml -> Skaffold.Configuration struct
@ericzzzzzzz ericzzzzzzz added the kokoro:force-run forces a kokoro re-run on a PR label Apr 19, 2024
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Apr 19, 2024
@ericzzzzzzz ericzzzzzzz requested review from a team and renzodavid9 April 19, 2024 18:47
@ericzzzzzzz ericzzzzzzz force-pushed the support-templating branch 3 times, most recently from f930ceb to 3e5171d Compare April 19, 2024 20:36
Comment thread pkg/skaffold/tags/templates.go
@ericzzzzzzz ericzzzzzzz force-pushed the support-templating branch from c9e2f7c to 47e8491 Compare May 2, 2024 19:57
@codecov

codecov Bot commented May 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 79.56989% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 63.10%. Comparing base (290280e) to head (47e8491).
Report is 1151 commits behind head on main.

Files Patch % Lines
pkg/skaffold/tags/templates.go 81.60% 9 Missing and 7 partials ⚠️
cmd/skaffold/app/cmd/diagnose.go 25.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9393      +/-   ##
==========================================
- Coverage   70.48%   63.10%   -7.38%     
==========================================
  Files         515      645     +130     
  Lines       23150    33166   +10016     
==========================================
+ Hits        16317    20931    +4614     
- Misses       5776    10611    +4835     
- Partials     1057     1624     +567     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ericzzzzzzz ericzzzzzzz merged commit c0bf1b6 into GoogleContainerTools:main May 3, 2024
@menahyouyeah menahyouyeah mentioned this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants