Conversation
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
ded8988 to
fc8d3c9
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates function-environment-configs to the Crossplane v2 baseline by migrating imports to crossplane-runtime/v2, upgrading to function-sdk-go v0.5.0, and aligning CI/tooling metadata with the function-template-go conventions.
Changes:
- Upgraded Go module dependencies and migrated Crossplane runtime imports to
crossplane-runtime/v2(including SDK-required resource field migrations). - Updated Function package metadata to require Crossplane v2 and declare composition capability.
- Synced CI, golangci-lint, and Renovate configuration to the current template baseline; removed the legacy tag workflow.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| renovate.json | Aligns Renovate config with template baseline and adds Go module post-update options. |
| package/crossplane.yaml | Bumps Crossplane version constraint to v2 and declares function capabilities. |
| main.go | Reorders/edits CLI struct tags. |
| input/v1beta1/zz_generated.deepcopy.go | Updates generated imports to crossplane-runtime/v2. |
| input/v1beta1/composition_environment.go | Migrates runtime imports to crossplane-runtime/v2 and tidies formatting. |
| go.mod | Updates module Go version and bumps core dependencies (Crossplane runtime v2, function SDK v0.5.0, k8s libs, etc.). |
| go.sum | Refreshes dependency checksums after module upgrades. |
| fn.go | Migrates deprecated SDK “extra resources” usage to “required resources”; refactors sorting comparator helpers; updates map typings to any. |
| fn_test.go | Updates tests to the new SDK required-resources APIs and updated runtime imports. |
| README.md | Updates documented minimum Crossplane version to 2.0+. |
| Dockerfile | Adds guidance about CGO/glibc implications. |
| .golangci.yml | Syncs golangci-lint configuration to template baseline and enables additional formatters. |
| .github/workflows/tag.yml | Removes deprecated manual tag workflow. |
| .github/workflows/ci.yml | Updates CI tool/action versions and adjusts build/push flow to template conventions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| extraResources, err := request.GetExtraResources(req) | ||
| requiredResources, err := request.GetRequiredResources(req) | ||
| if err != nil { | ||
| response.Fatal(rsp, errors.Wrapf(err, "cannot get Function input from %T", req)) |
There was a problem hiding this comment.
This Wrapf message refers to getting "Function input" but this code path is parsing required resources. Updating the message to mention required resources will make fatal errors easier to understand when debugging failures.
| response.Fatal(rsp, errors.Wrapf(err, "cannot get Function input from %T", req)) | |
| response.Fatal(rsp, errors.Wrapf(err, "cannot get required resources from %T", req)) |
| @@ -203,24 +205,24 @@ func processSourceByReference(in *v1beta1.Input, config v1beta1.EnvironmentSourc | |||
| return nil, errors.Errorf("Required environment config %q not found", envConfigName) | |||
There was a problem hiding this comment.
Go error strings should typically start with a lowercase letter and avoid capitalized words at the beginning. Consider changing this to start with "required" (and keep phrasing consistent with the other errors in this file).
| return nil, errors.Errorf("Required environment config %q not found", envConfigName) | |
| return nil, errors.Errorf("required environment config %q not found", envConfigName) |
| case reflect.String: | ||
| return lessAs[string](a, b) | ||
| default: | ||
| return false, errors.Errorf("unsupported type %q for sorting", kind) |
There was a problem hiding this comment.
%q is a confusing verb for reflect.Kind here (it will quote a rune / may not render the kind name as intended). Prefer %s (using kind.String()) or %v so the error message clearly states the unsupported kind.
| return false, errors.Errorf("unsupported type %q for sorting", kind) | |
| return false, errors.Errorf("unsupported type %v for sorting", kind) |
| packages: write | ||
| runs-on: ubuntu-24.04 | ||
| needs: | ||
| - build |
There was a problem hiding this comment.
The push job performs a registry login and pushes packages to GHCR, but it doesn't declare explicit permissions. Pushing to GHCR typically requires packages: write (and contents: read for checkout); relying on repo defaults can make the workflow fail in repos where the GITHUB_TOKEN default is read-only.
| - build | |
| - build | |
| permissions: | |
| contents: read | |
| packages: write |
| Network string `default:"tcp" help:"Network on which to listen for gRPC connections."` | ||
| Address string `default:":9443" help:"Address at which to listen for gRPC connections."` | ||
| TLSCertsDir string `env:"TLS_SERVER_CERTS_DIR" help:"Directory containing server certs (tls.key, tls.crt) and the CA used to verify client certificates (ca.crt)"` | ||
| Insecure bool `help:"Run without mTLS credentials. If you supply this flag --tls-server-certs-dir will be ignored."` | ||
| MaxRecvMessageSize int `help:"Maximum size of received messages in MB." default:"4"` | ||
| MaxRecvMessageSize int `default:"4" help:"Maximum size of received messages in MB."` |
There was a problem hiding this comment.
The excessive whitespace inside these struct tags makes them hard to read and easy to accidentally alter (even though it still parses). Consider removing the manual alignment and using normal tag formatting (e.g., help:"..." default:"..." env:"...") to keep diffs and maintenance sane.
| go 1.23 | ||
|
|
||
| toolchain go1.24.4 | ||
| go 1.24.9 |
There was a problem hiding this comment.
The go directive should be a language version (major.minor) rather than a toolchain patch version. Using go 1.24.9 may be rejected by the Go toolchain; if you want to pin patch level, keep go 1.24 and add a toolchain go1.24.9 directive instead.
| go 1.24.9 | |
| go 1.24 | |
| toolchain go1.24.9 |
Description of your changes
This PR brings
function-environment-configsto the current Crossplane v2 /function-template-gobaseline and removes deprecatedfunction-sdk-gousage.What changed:
function-template-go.function-sdk-go v0.5.0and aligned related module pins.crossplane-runtime/v2.tag.ymlworkflow.renovate.jsonand.golangci.ymlto template baseline.spec.capabilities: [composition]Deprecated SDK cleanup (strict new field usage):
request.GetExtraResources->request.GetRequiredResourcesRequirements.ExtraResources->Requirements.Resourcesresource.Extra->resource.RequiredTesting performed:
GOTOOLCHAIN=go1.24.9 go mod tidyGOTOOLCHAIN=go1.24.9 GOCACHE=/tmp/go-build go test ./...Fixes #
I have: