diff --git a/AGENTS.md b/AGENTS.md index bd8ad80a..a1725e51 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 4b6b5a7a..a13802f1 100644 --- a/pkg/cmds/list_feature_charts.go +++ b/pkg/cmds/list_feature_charts.go @@ -19,23 +19,26 @@ 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 + excludeCharts []string ) cmd := &cobra.Command{ Use: "list-feature-charts", @@ -43,29 +46,71 @@ 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(excludeFeatureCharts(charts, excludeCharts)) + 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") + 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 { @@ -82,12 +127,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 +167,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 +188,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 00000000..12889d20 --- /dev/null +++ b/pkg/lib/feature.go @@ -0,0 +1,148 @@ +/* +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" + "strings" + "sync" + + "kmodules.xyz/go-containerregistry/name" + + 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 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) { + // 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 b806dc92..245cb7e1 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" @@ -25,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" @@ -73,24 +73,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,22 +96,85 @@ 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(`\$\{[^}]*\}`) +// 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" { @@ -131,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 {