From 4111dd729098b1bc3385f71c90212744b4ed2f48 Mon Sep 17 00:00:00 2001 From: Arnob Kumar Saha Date: Sun, 16 Aug 2026 21:43:44 +0600 Subject: [PATCH 1/4] Collect images referenced by feature charts list-feature-charts recorded only the chart OCI refs pinned by each Feature/FeatureSet, and nothing ever rendered those charts, so the container images they deploy reached no catalog. list --root-dir=charts cannot cover them either: in the installer tree those images appear only inside a Feature's spec.values as maps (image.repository, deployment.image.name), while collectImages records string values under an "image" key, and the tag is absent entirely -- it comes from the feature chart's own appVersion. Render each feature chart at its pinned version using the values its Feature carries, and write the images they reference to feature-chart-images.yaml. Charts that fail to render are reported by name rather than aborting the run, so one broken chart cannot silently empty the catalog. Against appscode-cloud/installer this collects 546 images from 114 charts, among them ghcr.io/stakater/reloader and registry.k8s.io/prometheus-adapter/prometheus-adapter, which no catalog listed before. feature-charts.yaml output is unchanged. Signed-off-by: Arnob Kumar Saha --- AGENTS.md | 2 +- pkg/cmds/list_feature_charts.go | 67 ++++++++++++++---- pkg/lib/feature.go | 122 ++++++++++++++++++++++++++++++++ pkg/lib/image.go | 59 +++++++++------ 4 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 pkg/lib/feature.go diff --git a/AGENTS.md b/AGENTS.md index bd8ad80a8..a1725e510 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ Go module `kmodules.xyz/image-packer` — a CLI of OCI image tools used by AppsC - `parse` — parse an image reference into registry/repo/tag/digest components. - `list` — list all images referenced by a Helm chart. - `list-editor-charts` — list editor charts (catalog mode). -- `list-feature-charts` — list feature-set charts. +- `list-feature-charts` — list feature-set charts (`feature-charts.yaml`) and, unless `--with-images=false`, render each of them at its pinned version with the values its Feature carries to list the container images they reference (`feature-chart-images.yaml`). The second output exists because feature charts are third-party — their images appear in the installer tree only as `image.repository` / `image.name` maps inside a Feature's `spec.values`, which `list` cannot see, and without a tag. - `ace-up` — upload the ACE catalog images to a target registry. - `generate-scripts` — generate `copy-images.sh` / `export-images.sh` / `import-images.sh` / `import-into-k3s.sh` for an image catalog. - `generate-gcp-script` — GCP-flavored mirror script. diff --git a/pkg/cmds/list_feature_charts.go b/pkg/cmds/list_feature_charts.go index 4b6b5a7a7..cbfbc2c99 100644 --- a/pkg/cmds/list_feature_charts.go +++ b/pkg/cmds/list_feature_charts.go @@ -19,23 +19,25 @@ package cmds import ( "encoding/json" "errors" - "fmt" - "os" "path/filepath" + "sort" + "strings" "kmodules.xyz/client-go/tools/parser" + "kmodules.xyz/image-packer/pkg/lib" "github.com/spf13/cobra" shell "gomodules.xyz/go-sh" - "gopkg.in/yaml.v2" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" ) func NewCmdListFeatureCharts() *cobra.Command { var ( - rootDir string - outDir string + rootDir string + outDir string + withImages = true ) cmd := &cobra.Command{ Use: "list-feature-charts", @@ -43,24 +45,38 @@ func NewCmdListFeatureCharts() *cobra.Command { DisableFlagsInUseLine: true, DisableAutoGenTag: true, RunE: func(cmd *cobra.Command, args []string) error { - images, err := ListUICharts(rootDir) + charts, err := ListUICharts(rootDir) if err != nil { return err } - data, err := yaml.Marshal(images) - if err != nil { + refs := sets.New[string]() + for _, chart := range charts { + refs.Insert(chart.Ref()) + } + if err := write(sets.List(refs), filepath.Join(outDir, "feature-charts.yaml")); err != nil { return err } - filename := filepath.Join(outDir, "feature-charts.yaml") - err = os.WriteFile(filename, data, 0o644) - return err + if !withImages { + return nil + } + + images, skipped, err := lib.FeatureChartImages(charts) + if err != nil { + return err + } + if len(skipped) > 0 { + klog.Warningf("%d feature chart(s) failed to render; their images are missing from feature-chart-images.yaml: %s", + len(skipped), strings.Join(skipped, ", ")) + } + return write(images, filepath.Join(outDir, "feature-chart-images.yaml")) }, } cmd.Flags().StringVar(&rootDir, "root-dir", "", "Root directory") cmd.Flags().StringVar(&outDir, "output-dir", "", "Output directory") + cmd.Flags().BoolVar(&withImages, "with-images", withImages, "Render each feature chart and write the images it references to feature-chart-images.yaml") _ = cobra.MarkFlagRequired(cmd.Flags(), "output-dir") return cmd @@ -82,12 +98,11 @@ type ChartInfo struct { Description string `json:"description"` } -func ListUICharts(rootDir string) ([]string, error) { +func ListUICharts(rootDir string) ([]lib.FeatureChart, error) { sh := shell.NewSession() sh.SetDir("/tmp") sh.ShowCMD = true - images := sets.New[string]() var out []byte var err error @@ -123,6 +138,10 @@ func ListUICharts(rootDir string) ([]string, error) { panic(err) } + var charts []lib.FeatureChart + // A chart can be pinned by several Features; the same chart deployed with + // different values can pull different images, so dedup on values too. + seen := sets.New[string]() for _, ri := range helmout { if ri.Object.GetKind() != "FeatureSet" && ri.Object.GetKind() != "Feature" { continue @@ -140,9 +159,27 @@ func ListUICharts(rootDir string) ([]string, error) { } else if !found { continue } + values, _, err := unstructured.NestedMap(ri.Object.UnstructuredContent(), "spec", "values") + if err != nil { + return nil, err + } - images.Insert(fmt.Sprintf("ghcr.io/appscode-charts/%s:%s", chartName, chartVersion)) + chart := lib.FeatureChart{ + Name: chartName, + Version: chartVersion, + Values: values, + } + key, err := json.Marshal([]any{chart.Ref(), values}) + if err != nil { + return nil, err + } + if seen.Has(string(key)) { + continue + } + seen.Insert(string(key)) + charts = append(charts, chart) } - return sets.List(images), nil + sort.Slice(charts, func(i, j int) bool { return charts[i].Ref() < charts[j].Ref() }) + return charts, nil } diff --git a/pkg/lib/feature.go b/pkg/lib/feature.go new file mode 100644 index 000000000..67b1f323a --- /dev/null +++ b/pkg/lib/feature.go @@ -0,0 +1,122 @@ +/* +Copyright AppsCode Inc. and Contributors + +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 lib + +import ( + "fmt" + "maps" + "os" + "sort" + "sync" + + shell "gomodules.xyz/go-sh" + "k8s.io/klog/v2" + "sigs.k8s.io/yaml" +) + +const ( + FeatureChartRegistry = "ghcr.io/appscode-charts" + + featureChartWorkers = 8 +) + +// FeatureChart is a chart pinned by a Feature or FeatureSet, along with the +// values that resource deploys it with. +type FeatureChart struct { + Name string + Version string + Values map[string]any +} + +func (c FeatureChart) Ref() string { + return fmt.Sprintf("%s/%s:%s", FeatureChartRegistry, c.Name, c.Version) +} + +// FeatureChartImages renders each feature chart at its pinned version with the +// values its Feature carries and returns the images they reference. Charts that +// fail to render are reported in skipped rather than failing the whole run, so +// that one broken chart can't silently empty the catalog. +func FeatureChartImages(charts []FeatureChart) (images []string, skipped []string, err error) { + found := map[string]string{} + + var mu sync.Mutex + var wg sync.WaitGroup + sem := make(chan struct{}, featureChartWorkers) + + for _, chart := range charts { + wg.Add(1) + go func() { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + + chartImages, err := featureChartImages(chart) + + mu.Lock() + defer mu.Unlock() + if err != nil { + klog.Infof("Skipping feature chart %s due to error: %v", chart.Ref(), err) + skipped = append(skipped, chart.Ref()) + return + } + maps.Copy(found, chartImages) + }() + } + wg.Wait() + + sort.Strings(skipped) + return ListImages(found), skipped, nil +} + +func featureChartImages(chart FeatureChart) (map[string]string, error) { + // No release name: some feature chart names are longer than helm's 53 char + // release name limit, and the generated name has no bearing on images. + args := []any{ + "template", + fmt.Sprintf("oci://%s/%s", FeatureChartRegistry, chart.Name), + "--version=" + chart.Version, + } + + if len(chart.Values) > 0 { + content, err := yaml.Marshal(chart.Values) + if err != nil { + return nil, err + } + filename, err := writeTempValues(chart.Name, content) + if err != nil { + return nil, err + } + defer os.Remove(filename) // nolint:errcheck + + args = append(args, "--values="+filename) + } + + sh := shell.NewSession() + sh.SetDir(os.TempDir()) + sh.ShowCMD = true + + out, err := sh.Command("helm", args...).Output() + if err != nil { + return nil, err + } + + images := map[string]string{} + if err := CollectRenderedImages(out, images); err != nil { + return nil, err + } + return images, nil +} diff --git a/pkg/lib/image.go b/pkg/lib/image.go index b806dc922..834fe7f4f 100644 --- a/pkg/lib/image.go +++ b/pkg/lib/image.go @@ -17,7 +17,6 @@ limitations under the License. package lib import ( - "io" "os" "path/filepath" "regexp" @@ -73,24 +72,13 @@ func mapChartImages(rootDir string, values map[string]string, sh *shell.Session, content, ok := values[chartName] if ok { - tmpfile, err := os.CreateTemp("", chartName+"-val-*.yaml") + filename, err := writeTempValues(chartName, []byte(content)) if err != nil { klog.Fatal(err) } - defer os.Remove(tmpfile.Name()) // nolint:errcheck + defer os.Remove(filename) // nolint:errcheck - if _, err := io.WriteString(tmpfile, content); err != nil { - tmpfile.Close() // nolint:errcheck - klog.Fatal(err) - } - - // 4. Close the file handle - // We must close the file handle before attempting to read from it or before the defer os.Remove runs. - if err := tmpfile.Close(); err != nil { - klog.Fatal(err) - } - - args = append(args, "--values="+tmpfile.Name()) + args = append(args, "--values="+filename) } if _, err := os.Stat(filepath.Join(rootDir, chartName, "ci", "ci-values.yaml")); err == nil { @@ -107,19 +95,48 @@ func mapChartImages(rootDir string, values map[string]string, sh *shell.Session, } } if out, err := sh.SetDir(rootDir).Command("helm", args...).Output(); err == nil { - helmout, err := parser.ListResources(out) - if err != nil { + if err := CollectRenderedImages(out, images); err != nil { panic(err) } - - for _, ri := range helmout { - collectImages(ri.Object.UnstructuredContent(), images, ri.Object.GetObjectKind().GroupVersionKind().GroupKind().String()) - } } else { klog.Infof("Skipping %s due to error: %v", chartName, err) } } +func writeTempValues(chartName string, content []byte) (string, error) { + tmpfile, err := os.CreateTemp("", chartName+"-val-*.yaml") + if err != nil { + return "", err + } + + if _, err := tmpfile.Write(content); err != nil { + tmpfile.Close() // nolint:errcheck + os.Remove(tmpfile.Name()) // nolint:errcheck + return "", err + } + + // The handle must be closed before helm reads the file. + if err := tmpfile.Close(); err != nil { + os.Remove(tmpfile.Name()) // nolint:errcheck + return "", err + } + return tmpfile.Name(), nil +} + +// CollectRenderedImages records every image referenced by the resources in a +// `helm template` output into images, keyed by image and valued by the GroupKind +// of the resource that referenced it. +func CollectRenderedImages(out []byte, images map[string]string) error { + resources, err := parser.ListResources(out) + if err != nil { + return err + } + for _, ri := range resources { + collectImages(ri.Object.UnstructuredContent(), images, ri.Object.GetObjectKind().GroupVersionKind().GroupKind().String()) + } + return nil +} + // placeholderRE matches a shell-style ${...} template placeholder. var placeholderRE = regexp.MustCompile(`\$\{[^}]*\}`) From 7787c39bb3f38d0a6c9da9d04a18a651f6089027 Mon Sep 17 00:00:00 2001 From: Arnob Kumar Saha Date: Sun, 16 Aug 2026 21:49:49 +0600 Subject: [PATCH 2/4] Drop digest-only refs from the feature chart image list generate-scripts derives a tarball name and a destination reference from an image's tag, so it errors out on a reference carrying only a digest. Feature charts occasionally pin that way -- secrets-store-csi-driver- provider-gcp does -- and a single such ref would fail the whole update-catalog run. Drop those refs and name them in a warning, so the gap is visible rather than silent. Signed-off-by: Arnob Kumar Saha --- pkg/lib/feature.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pkg/lib/feature.go b/pkg/lib/feature.go index 67b1f323a..12889d20e 100644 --- a/pkg/lib/feature.go +++ b/pkg/lib/feature.go @@ -21,8 +21,11 @@ import ( "maps" "os" "sort" + "strings" "sync" + "kmodules.xyz/go-containerregistry/name" + shell "gomodules.xyz/go-sh" "k8s.io/klog/v2" "sigs.k8s.io/yaml" @@ -79,7 +82,30 @@ func FeatureChartImages(charts []FeatureChart) (images []string, skipped []strin wg.Wait() sort.Strings(skipped) - return ListImages(found), skipped, nil + return dropUntaggedImages(ListImages(found)), skipped, nil +} + +// dropUntaggedImages removes digest-only references. generate-scripts derives a +// tarball name and a destination reference from the tag, so it rejects images +// that carry only a digest; keeping them would break every catalog that feeds +// on this list. Feature charts do occasionally pin by digest, so drop those +// refs loudly rather than let the whole run fail. +func dropUntaggedImages(images []string) []string { + result := make([]string, 0, len(images)) + var dropped []string + for _, img := range images { + ref, err := name.ParseReference(img) + if err != nil || ref.Tag == "" { + dropped = append(dropped, img) + continue + } + result = append(result, img) + } + if len(dropped) > 0 { + klog.Warningf("dropping %d digest-only image ref(s), not mirrorable by generate-scripts: %s", + len(dropped), strings.Join(dropped, ", ")) + } + return result } func featureChartImages(chart FeatureChart) (map[string]string, error) { From 9a34a36052d690c9b68f053c5b0c3c91d8b4a210 Mon Sep 17 00:00:00 2001 From: Arnob Kumar Saha Date: Sun, 16 Aug 2026 22:15:54 +0600 Subject: [PATCH 3/4] Add --exclude-chart to list-feature-charts Most feature charts belong to an installer that already publishes its own catalog/imagelist.yaml, so rendering every chart makes feature-chart-images.yaml restate images another catalog owns -- for appscode-cloud/installer that was 398 of 545 entries, and it would mirror a second copy of every database image. Which charts those are is deployment policy, not something this tool can know, so take the names from the caller. An exclusion matching no chart is reported: a stale or misspelled entry would otherwise silently start collecting images again. feature-charts.yaml is unaffected -- the charts themselves still need mirroring. Signed-off-by: Arnob Kumar Saha --- pkg/cmds/list_feature_charts.go | 37 +++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/cmds/list_feature_charts.go b/pkg/cmds/list_feature_charts.go index cbfbc2c99..a13802f15 100644 --- a/pkg/cmds/list_feature_charts.go +++ b/pkg/cmds/list_feature_charts.go @@ -35,9 +35,10 @@ import ( func NewCmdListFeatureCharts() *cobra.Command { var ( - rootDir string - outDir string - withImages = true + rootDir string + outDir string + withImages = true + excludeCharts []string ) cmd := &cobra.Command{ Use: "list-feature-charts", @@ -62,7 +63,7 @@ func NewCmdListFeatureCharts() *cobra.Command { return nil } - images, skipped, err := lib.FeatureChartImages(charts) + images, skipped, err := lib.FeatureChartImages(excludeFeatureCharts(charts, excludeCharts)) if err != nil { return err } @@ -77,11 +78,39 @@ func NewCmdListFeatureCharts() *cobra.Command { cmd.Flags().StringVar(&rootDir, "root-dir", "", "Root directory") cmd.Flags().StringVar(&outDir, "output-dir", "", "Output directory") cmd.Flags().BoolVar(&withImages, "with-images", withImages, "Render each feature chart and write the images it references to feature-chart-images.yaml") + cmd.Flags().StringSliceVar(&excludeCharts, "exclude-chart", nil, "Feature charts to leave out of feature-chart-images.yaml, by chart name. Use for charts whose images another catalog already publishes. Does not affect feature-charts.yaml") _ = cobra.MarkFlagRequired(cmd.Flags(), "output-dir") return cmd } +func excludeFeatureCharts(charts []lib.FeatureChart, exclude []string) []lib.FeatureChart { + if len(exclude) == 0 { + return charts + } + + skip := sets.New[string](exclude...) + kept := make([]lib.FeatureChart, 0, len(charts)) + matched := sets.New[string]() + for _, chart := range charts { + if skip.Has(chart.Name) { + matched.Insert(chart.Name) + continue + } + kept = append(kept, chart) + } + + // An exclusion that matches nothing is a stale or misspelled entry in the + // caller's list, and it would silently start collecting images again. + if unmatched := skip.Difference(matched); unmatched.Len() > 0 { + klog.Warningf("%d --exclude-chart value(s) matched no feature chart: %s", + unmatched.Len(), strings.Join(sets.List(unmatched), ", ")) + } + klog.Infof("excluded %d feature chart(s) from the image list", matched.Len()) + + return kept +} + type Skeleton struct { Spec struct { Chart struct { From 7a6b3d8ae66502140597b07f621c2ca306639887 Mon Sep 17 00:00:00 2001 From: Arnob Kumar Saha Date: Sun, 16 Aug 2026 22:40:17 +0600 Subject: [PATCH 4/4] Collect images passed to containers as flags An operator that launches other workloads takes their image as a flag -- --acme-http01-solver-image, --prometheus-config-reloader, --thanos-default-base-image -- and that reference appears nowhere else in the manifest, so it reached no catalog. appscode-cloud/artifacts carried a regex over rendered output to recover three such images; collecting them here retires that. name.ParseReference cannot be the test on its own: it defaults the registry to docker.io and the tag to latest, so --log-level=info parses as an image. Demand an explicit registry host and an explicit tag or digest, which an image passed this way always carries. Verified against appscode-cloud/installer: catalog/imagelist.yaml is byte-identical, so the scan adds no false positives, and the three images above now appear in feature-chart-images.yaml. Signed-off-by: Arnob Kumar Saha --- pkg/lib/image.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/lib/image.go b/pkg/lib/image.go index 834fe7f4f..245cb7e1e 100644 --- a/pkg/lib/image.go +++ b/pkg/lib/image.go @@ -24,6 +24,7 @@ import ( "strings" "kmodules.xyz/client-go/tools/parser" + "kmodules.xyz/go-containerregistry/name" shell "gomodules.xyz/go-sh" "k8s.io/apimachinery/pkg/runtime/schema" @@ -140,6 +141,40 @@ func CollectRenderedImages(out []byte, images map[string]string) error { // placeholderRE matches a shell-style ${...} template placeholder. var placeholderRE = regexp.MustCompile(`\$\{[^}]*\}`) +// containerArgRE matches a --flag=value container argument. +var containerArgRE = regexp.MustCompile(`^--[A-Za-z0-9._-]+=(\S+)$`) + +// imageFromContainerArg reports an image reference passed to a container as a +// flag, e.g. --acme-http01-solver-image= or --prometheus-config-reloader=. +// An operator that launches other workloads takes their image this way, and the +// reference appears nowhere else in the manifest. +func imageFromContainerArg(arg string) (string, bool) { + m := containerArgRE.FindStringSubmatch(arg) + if m == nil { + return "", false + } + ref := m[1] + + // name.ParseReference is far too permissive on its own: it defaults the + // registry to docker.io and the tag to latest, so it accepts most flag + // values (--log-level=info parses). Demand an explicit registry host and an + // explicit tag or digest, which every image passed this way carries. + if strings.Contains(ref, "://") { + return "", false + } + host, remainder, ok := strings.Cut(ref, "/") + if !ok || (!strings.ContainsAny(host, ".:") && host != "localhost") { + return "", false + } + if last := remainder[strings.LastIndex(remainder, "/")+1:]; !strings.ContainsAny(last, ":@") { + return "", false + } + if _, err := name.ParseReference(ref); err != nil { + return "", false + } + return ref, true +} + func collectImages(obj map[string]any, images map[string]string, srcGK string) { for k, v := range obj { if k == "image" { @@ -148,6 +183,16 @@ func collectImages(obj map[string]any, images map[string]string, srcGK string) { images[img] = srcGK } } + } else if k == "args" || k == "command" { + if items, ok := v.([]any); ok { + for _, item := range items { + if s, ok := item.(string); ok { + if img, ok := imageFromContainerArg(s); ok { + images[img] = srcGK + } + } + } + } } else if m, ok := v.(map[string]any); ok { collectImages(m, images, srcGK) } else if items, ok := v.([]any); ok {