Version pinning for templates #1012
AI Code Review Results
AI Pull Request Overview
Summary
- Introduces template version pinning via @tag, @Branch, or @sha-xxx in function lang field
- Supports pinning for both store and custom template sources
- Adds versioncontrol package for git reference parsing
- Implements SHA pinning with full repository cloning
- Includes test configurations demonstrating pinned templates
Summary per file
Summary per file
| File path | Summary |
|---|---|
| commands/new_function.go | Parses @ref from language and converts to # for template pulls |
| commands/fetch_templates.go | Adds SHA ref handling with full clone and git checkout |
| commands/template_pull_stack.go | Updates to pull templates from stack configuration |
| versioncontrol/core.go | Defines git clone commands for different ref types |
| versioncontrol/git.go | Implements git operations for cloning and SHA retrieval |
| versioncontrol/parse.go | Parses pinned git URLs with # fragments |
| versioncontrol/parse_test.go | Tests for ref parsing functionality |
| testing/template-config.yaml | Test stack config with @ pinned languages |
| testing/go-090/, testing/go-latest/, testing/go-sha-2e6e262/ | Test function directories for different pin types |
| Other modified files | Minor updates to template fetching and validation |
Overall Assessment
Overall Assessment
The PR successfully adds template versioning functionality, addressing the risk of unexpected breaking changes in templates. The implementation provides three pinning methods (tag/branch, SHA) and integrates well with existing store and custom source workflows. The versioncontrol package adds robust git ref handling. However, the implementation has critical gaps in custom source pinning support, leading to inconsistent behavior and potential runtime failures when @ refs are used with configured template sources.
Detailed Review
Detailed Review
Custom Source Pinning Logic Flaws
In commands/template_pull_stack.go, the pullStackTemplates function fails to properly handle @ refs when custom template sources are configured. The matching logic if config.Name == val cannot succeed because val (from function.Language) includes the @ref suffix while config.Name does not. This causes the code to fall back to store pulls even when a custom source is specified, defeating the purpose of custom configurations.
For example, with:
configuration:
templates:
- name: golang-middleware
source: https://github.com/openfaas/golang-http-template
functions:
go-090:
lang: golang-middleware@0.9.0The function will attempt to pull from the store instead of applying the @0.9.0 ref to the custom source.
Directory Path Resolution Issues
commands/fetch_templates.go moveTemplates function does not consistently append ref suffixes to destination directories. When templateName is provided (specific template pull), languageDest is set to templateName without @ref, but functions with pinned langs expect directories like golang-middleware@0.9.0. This mismatch will cause template resolution failures during build/publish operations.
Incomplete @ to # Conversion
commands/fetch_templates.go pullTemplate function lacks logic to parse @ refs from templateName and convert them to # fragments in repository URLs. This prevents correct ref application for custom sources, as the parsing only handles # in URLs, not @ in template names.
Test Coverage Deficiencies
commands/template_pull_stack_test.go contains no test cases for @ ref handling in template names. The new testing/template-config.yaml demonstrates @ usage but relies on store fallback behavior rather than testing custom source pinning. This leaves critical code paths untested.
SHA Pinning Performance Impact
SHA pinning requires full repository cloning (GitCloneFullDepth), which is significantly slower and more resource-intensive than shallow clones used for tags/branches. While necessary for SHA access, this could impact CI/CD performance for large repositories. Consider documenting this trade-off.
Recommendations
-
Fix Custom Source Matching: Update
pullStackTemplatesto parse base names fromvaland match againstconfig.Name, then construct repository URLs with #ref for custom sources. -
Standardize Directory Naming: Modify
moveTemplatesto always append @ref tolanguageDestwhenrefNameis present, ensuring consistent directory structures. -
Add @ Parsing: Implement @ to # conversion in
pullTemplatefor templateName refs. -
Expand Test Coverage: Add test cases in
template_pull_stack_test.gofor @ ref scenarios with both store and custom sources. -
Documentation: Update CLI help and documentation to clarify the difference between @ (function-level) and # (source-level) pinning.
-
Error Handling: Add validation for invalid ref formats (e.g., non-hex SHA values) with clear error messages.
The core functionality is sound and the feature addresses a real user need. With the identified fixes, this will provide robust template versioning across all supported scenarios.
AI agent details.
Agent processing time: 1m27.809s
Environment preparation time: 6.284s
Total time from webhook: 1m38.521s