-
Notifications
You must be signed in to change notification settings - Fork 23
feat: allow composition render subcommand read from configuration pkg file #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -86,11 +86,12 @@ type Cmd struct { | |||||||||||||||||||||||||||||||||||||||||
| FunctionCredentials string `help:"A YAML file or directory of YAML files specifying credentials to use for Functions to render the XR." placeholder:"PATH" predictor:"yaml_file_or_directory" type:"path"` | ||||||||||||||||||||||||||||||||||||||||||
| FunctionAnnotations []string `help:"Override function annotations for all functions. Provide multiple annotations by repeating the argument." placeholder:"KEY=VALUE" short:"a"` | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| CacheDir string `env:"CROSSPLANE_XPKG_CACHE" help:"Directory for cached xpkg package contents." name:"cache-dir"` | ||||||||||||||||||||||||||||||||||||||||||
| CacheDir string `env:"CROSSPLANE_XPKG_CACHE" help:"Directory for cached xpkg package contents." name:"cache-dir"` | ||||||||||||||||||||||||||||||||||||||||||
| MaxConcurrency uint `default:"8" help:"Maximum concurrency for building embedded functions."` | ||||||||||||||||||||||||||||||||||||||||||
| ProjectFile string `default:"crossplane-project.yaml" help:"Path to the project file. Optional." optional:"" predictor:"yaml_file" short:"f" type:"path"` | ||||||||||||||||||||||||||||||||||||||||||
| PkgMetaFile string `default:"crossplane.yaml" help:"Path to a package metadata file (crossplane.yaml). Used as fallback when no project file is found." name:"pkg-meta-file" optional:"" predictor:"yaml_file" type:"path"` | ||||||||||||||||||||||||||||||||||||||||||
| ProjectFile string `default:"crossplane-project.yaml" help:"Path to the project file. Optional." optional:"" predictor:"yaml_file" short:"f" type:"path"` | ||||||||||||||||||||||||||||||||||||||||||
| Timeout time.Duration `default:"1m" help:"How long to run before timing out."` | ||||||||||||||||||||||||||||||||||||||||||
| XRD string `help:"A YAML file specifying the CompositeResourceDefinition (XRD) that defines the XR's schema and properties." optional:"" placeholder:"PATH" type:"existingfile"` | ||||||||||||||||||||||||||||||||||||||||||
| XRD string `help:"A YAML file specifying the CompositeResourceDefinition (XRD) that defines the XR's schema and properties." optional:"" placeholder:"PATH" type:"existingfile"` | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| fs afero.Fs | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -405,7 +406,7 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal | |||||||||||||||||||||||||||||||||||||||||
| projDir := filepath.Dir(projFilePath) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if _, err := os.Stat(projFilePath); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.New("functions argument is required when not in a project") | ||||||||||||||||||||||||||||||||||||||||||
| return c.loadFunctionsFromConfiguration(ctx, log) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
408
to
410
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Handle non-not-found file errors before fallback.
Only use fallback behavior when
Proposed fix if _, err := os.Stat(projFilePath); err != nil {
+ if !os.IsNotExist(err) {
+ return nil, errors.Wrapf(err, "cannot access project file %q", projFilePath)
+ }
return c.loadFunctionsFromConfiguration(ctx, log)
}
if _, err := os.Stat(cfgFilePath); err != nil {
+ if !os.IsNotExist(err) {
+ return nil, errors.Wrapf(err, "cannot access configuration file %q", cfgFilePath)
+ }
return nil, errors.New("functions argument is required when not in a project or configuration")
}📝 Committable suggestion
Suggested change
Suggested change
📍 Affects 1 file
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| log.Debug("Loading functions from project", "project-file", projFilePath) | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -493,3 +494,45 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return fns, nil | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (c *Cmd) loadFunctionsFromConfiguration(ctx context.Context, log logging.Logger) ([]pkgv1.Function, error) { | ||||||||||||||||||||||||||||||||||||||||||
| cfgFilePath, err := filepath.Abs(c.PkgMetaFile) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.Wrap(err, "cannot determine configuration file path") | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if _, err := os.Stat(cfgFilePath); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.New("functions argument is required when not in a project or configuration") | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| log.Debug("Loading functions from configuration file", "configuration-file", cfgFilePath) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| cfgDir := filepath.Dir(cfgFilePath) | ||||||||||||||||||||||||||||||||||||||||||
| cfgFS := afero.NewBasePathFs(afero.NewOsFs(), cfgDir) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| cfg, err := clixpkg.ParseConfiguration(cfgFS, filepath.Base(cfgFilePath)) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.Wrapf(err, "cannot parse configuration file %q", cfgFilePath) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| cacheDir := c.CacheDir | ||||||||||||||||||||||||||||||||||||||||||
| if cacheDir == "" { | ||||||||||||||||||||||||||||||||||||||||||
| cacheDir = dependency.DefaultCacheDir() | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| xpkgClient, err := clixpkg.NewClient( | ||||||||||||||||||||||||||||||||||||||||||
| clixpkg.NewRemoteFetcher(), | ||||||||||||||||||||||||||||||||||||||||||
| clixpkg.WithCacheDir(afero.NewOsFs(), cacheDir), | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.Wrap(err, "cannot create xpkg client") | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| resolver := clixpkg.NewResolver(xpkgClient) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| fns, err := clixpkg.ResolveConfigurationFunctions(ctx, cfg, resolver) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.Wrap(err, "cannot resolve function dependencies from configuration file") | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return fns, nil | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| Copyright 2026 The Crossplane Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package xpkg | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "path" | ||
|
|
||
| "github.com/spf13/afero" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "sigs.k8s.io/yaml" | ||
|
|
||
| "github.com/crossplane/crossplane-runtime/v2/pkg/errors" | ||
|
|
||
| pkgmetav1 "github.com/crossplane/crossplane/apis/v2/pkg/meta/v1" | ||
| pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" | ||
| ) | ||
|
|
||
| // ParseConfiguration parses a Configuration package metadata file and returns the Configuration. | ||
| func ParseConfiguration(fs afero.Fs, filePath string) (*pkgmetav1.Configuration, error) { | ||
| bs, err := afero.ReadFile(fs, filePath) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to read configuration file %q", filePath) | ||
| } | ||
|
|
||
| var tm metav1.TypeMeta | ||
| if err := yaml.Unmarshal(bs, &tm); err != nil { | ||
| return nil, errors.Wrap(err, "failed to parse configuration file") | ||
| } | ||
|
|
||
| wantAPIVersion := pkgmetav1.SchemeGroupVersion.String() | ||
| if tm.APIVersion != wantAPIVersion { | ||
| return nil, errors.Errorf("unsupported configuration apiVersion %q, expected %q", tm.APIVersion, wantAPIVersion) | ||
| } | ||
| if tm.Kind != pkgmetav1.ConfigurationKind { | ||
| return nil, errors.Errorf("unsupported configuration kind %q, expected %q", tm.Kind, pkgmetav1.ConfigurationKind) | ||
| } | ||
|
|
||
| var cfg pkgmetav1.Configuration | ||
| if err := yaml.Unmarshal(bs, &cfg); err != nil { | ||
| return nil, errors.Wrap(err, "failed to parse configuration file") | ||
| } | ||
|
|
||
| return &cfg, nil | ||
| } | ||
|
|
||
| // ResolveConfigurationFunctions extracts Function dependencies from a Configuration and resolves | ||
| // their version constraints to concrete OCI references. | ||
| func ResolveConfigurationFunctions(ctx context.Context, cfg *pkgmetav1.Configuration, resolver *Resolver) ([]pkgv1.Function, error) { | ||
| fns := make([]pkgv1.Function, 0, len(cfg.Spec.DependsOn)) | ||
| for _, dep := range cfg.Spec.DependsOn { | ||
| if dep.Function == nil { | ||
| continue | ||
| } | ||
|
|
||
| ref := *dep.Function | ||
| if dep.Version != "" { | ||
| ref = fmt.Sprintf("%s:%s", ref, dep.Version) | ||
| } | ||
|
|
||
| resolved, _, err := resolver.Resolve(ctx, ref) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "cannot resolve function dependency %q", ref) | ||
| } | ||
|
|
||
| fns = append(fns, pkgv1.Function{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: path.Base(resolved.Context().RepositoryStr()), | ||
| }, | ||
| Spec: pkgv1.FunctionSpec{ | ||
| PackageSpec: pkgv1.PackageSpec{ | ||
| Package: resolved.Name(), | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| return fns, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| /* | ||
| Copyright 2026 The Crossplane Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package xpkg | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
| "testing" | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| "github.com/spf13/afero" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
||
| pkgmetav1 "github.com/crossplane/crossplane/apis/v2/pkg/meta/v1" | ||
| pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" | ||
| ) | ||
|
|
||
| func TestParseConfiguration(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| content string | ||
| expectErr bool | ||
| }{ | ||
| { | ||
| name: "ValidConfiguration", | ||
| content: ` | ||
| apiVersion: meta.pkg.crossplane.io/v1 | ||
| kind: Configuration | ||
| metadata: | ||
| name: my-config | ||
| spec: | ||
| dependsOn: | ||
| - function: ghcr.io/example/function-a | ||
| version: "v1.0.0" | ||
| `, | ||
| }, | ||
| { | ||
| name: "WrongAPIVersion", | ||
| content: "apiVersion: wrong.api/v1\nkind: Configuration\nspec: {}", | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "WrongKind", | ||
| content: "apiVersion: meta.pkg.crossplane.io/v1\nkind: Provider\nspec: {}", | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "InvalidYAML", | ||
| content: "not: valid: yaml: [", | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "FileNotFound", | ||
| expectErr: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| fs := afero.NewMemMapFs() | ||
| if tt.content != "" { | ||
| if err := afero.WriteFile(fs, "/crossplane.yaml", []byte(tt.content), os.ModePerm); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| } | ||
|
|
||
| cfg, err := ParseConfiguration(fs, "/crossplane.yaml") | ||
| if (err != nil) != tt.expectErr { | ||
| t.Fatalf("ParseConfiguration() error = %v, expectErr %v", err, tt.expectErr) | ||
| } | ||
| if err == nil && cfg.Name != "my-config" { | ||
| t.Errorf("name = %q, want %q", cfg.Name, "my-config") | ||
| } | ||
| }) | ||
| } | ||
|
Comment on lines
+35
to
+93
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 3 'cmpopts\.EquateErrors|cmp\.Diff' --glob '*_test.go'
rg -n 'github.com/google/go-cmp' go.modRepository: crossplane/cli Length of output: 152 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== candidate file =="
fd -a 'internal/xpkg/configuration_test.go' . || true
if [ -f internal/xpkg/configuration_test.go ]; then
echo "== file outline =="
ast-grep outline internal/xpkg/configuration_test.go --view expanded || true
echo "== relevant lines =="
cat -n internal/xpkg/configuration_test.go | sed -n '1,180p'
fi
echo "== go-cmp in module files =="
rg -n 'go-cmp|cmpopts\.EquateErrors|cmp\.Diff' --glob '*_test.go' --glob 'go.mod' --glob 'go.sum' . || true
echo "== current diff stat/name =="
git diff --stat || true
git diff -- internal/xpkg/configuration_test.go 2>/dev/null | sed -n '1,220p' || trueRepository: crossplane/cli Length of output: 40017 Use the required
🤖 Prompt for AI AgentsSource: Path instructions |
||
| } | ||
|
|
||
| func TestResolveConfigurationFunctions(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| fnA := "ghcr.io/example/function-a" | ||
| fnB := "ghcr.io/example/function-b" | ||
| provider := "ghcr.io/example/provider-x" | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| deps []pkgmetav1.Dependency | ||
| want []pkgv1.Function | ||
| }{ | ||
| { | ||
| name: "FiltersFunctionsOnly", | ||
| deps: []pkgmetav1.Dependency{ | ||
| {Function: &fnA, Version: "v1.0.0"}, | ||
| {Provider: &provider, Version: "v2.0.0"}, | ||
| {Function: &fnB, Version: "v0.5.0"}, | ||
| }, | ||
| want: []pkgv1.Function{ | ||
| { | ||
| ObjectMeta: metav1.ObjectMeta{Name: "function-a"}, | ||
| Spec: pkgv1.FunctionSpec{PackageSpec: pkgv1.PackageSpec{Package: "ghcr.io/example/function-a:v1.0.0"}}, | ||
| }, | ||
| { | ||
| ObjectMeta: metav1.ObjectMeta{Name: "function-b"}, | ||
| Spec: pkgv1.FunctionSpec{PackageSpec: pkgv1.PackageSpec{Package: "ghcr.io/example/function-b:v0.5.0"}}, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "Empty", | ||
| deps: nil, | ||
| want: []pkgv1.Function{}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg := &pkgmetav1.Configuration{ | ||
| Spec: pkgmetav1.ConfigurationSpec{ | ||
| MetaSpec: pkgmetav1.MetaSpec{DependsOn: tt.deps}, | ||
| }, | ||
| } | ||
|
|
||
| resolver := NewResolver(&fakeClient{tags: []string{"v1.0.0", "v0.5.0", "latest"}}) | ||
| got, err := ResolveConfigurationFunctions(context.Background(), cfg, resolver) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if diff := cmp.Diff(tt.want, got); diff != "" { | ||
| t.Errorf("ResolveConfigurationFunctions (-want +got):\n%s", diff) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the positional argument help.
The
Functionshelp text says that the argument is optional only in a project. It is also optional when the Configuration metadata file supplies Function dependencies. Update the Kong help text to describe both cases.As per path instructions, “Review CLI commands for proper flag handling, help text, and error messages.”
🤖 Prompt for AI Agents
Source: Path instructions