Use common library reporter

[cblecker]

This uses the common library reporter template generation, so that we can use the DefaultFuncs in format output: https://github.com/containers/common/blob/main/pkg/report/template.go#L37

[mtrmac]

Thanks. Is this just for the extra functions?

• If so, why not just use DefaultFuncs directly? The other manipulations of the format string are not obviously desirable or harmless. (They might well be an improvement, but this PR doesn’t make an argument either way.)
• The new functions usable in the format string need to be documented somehow, otherwise will not be something users can rely on. That might well be a link to some other documentation that already exists, just not nothing.

[cblecker]

@mtrmac The goal here was to have output filters from podman images inspect --format work the same as skopeo inspect --format. I figured the fact that there was a common reporting library for use by the github.com/containers tools, and that other functions from that library are in use, that this was just a bug/oversight to not use this function.

The docs don't really outline these custom functions either way:
https://docs.podman.io/en/latest/markdown/podman-image-inspect.1.html
https://github.com/containers/skopeo/blob/main/docs/skopeo-inspect.1.md

The godoc has some documentation on it: https://pkg.go.dev/github.com/containers/[email protected]/pkg/report

I'm happy to add additional documentation somewhere if you'd like.

[mtrmac]

It’s more that it was never discussed (the history is #1070 ); so now we essentially get do define what needs to happen.

Speaking for Skopeo, “Go templates as they existing the Go standard library” is a simple story to tell (although that’s not quite what Skopeo does, sadly). Any other features do need to be documented somehow.

Linking to pkg.go.dev might be enough — if the c/common package at least formatted the documentation enough to allow linking to a specific section, and the list was a bit formatted; using the Go 1.19 documentation comment formatting might allow both of those, I’m not sure.

[cblecker]

Works for me.

I opened containers/common#1260 to get some deep links available on pkg.go.dev. Should I add the local details of this to https://github.com/containers/skopeo/blob/main/docs/skopeo-inspect.1.md ?

[mtrmac]

Thanks!

At the very least a link to the resulting page on pkg.go.dev would be nice. Alternatively, just duplicating the list of functions in a new section of the the skopeo inspect man page would work too — I guess the link to pkg.go.dev would be better, because it would be kept up-to-date.

[cblecker]

@mtrmac Updated PR with docs.

The anchor in the link won't work until containers/common#1260 is merged, but the format of it will be similar to https://pkg.go.dev/encoding/gob#hdr-Encoding_Details

[cblecker]

The common PR has been merged, and you can see the link works at https://pkg.go.dev/github.com/containers/common@main/pkg/report#hdr-Template_Functions

Once the common library cuts a new version, then it will be available at https://pkg.go.dev/github.com/containers/common/pkg/report#hdr-Template_Functions

[TomSweeneyRedHat]

@cblecker looks like you need a rebase here.

[cblecker]

@TomSweeneyRedHat Thanks for the ping. Rebased.

[TomSweeneyRedHat]

LGTM
thanks @cblecker !

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[cblecker]

@mtrmac @TomSweeneyRedHat I've resolved the merge conflict again. Is it possible to get this merged?

[TomSweeneyRedHat]

LGTM
@mtrmac ?

[rhatdan]

LGTM.

One of the things that @edsantiago has done in Podman is to a table to help users with --format options. From
man podman-container-inspect

  --format, -f=format
       Format the output using the given Go template.  The  keys  of  the  re‐
       turned  JSON can be used as the values for the --format flag (see exam‐
       ples below).

       Valid placeholders for the Go template are listed below:

       ┌─────────────────────┬───────────────────────────────┐
       │Placeholder          │ Description                   │
       ├─────────────────────┼───────────────────────────────┤
       │.AppArmorProfile     │ AppArmor profile (string)     │
       ├─────────────────────┼───────────────────────────────┤
       │.Args                │ Command-line arguments (array │
       │                     │ of strings)                   │
       ├─────────────────────┼───────────────────────────────┤
       │.BoundingCaps        │ Bounding  capability set (ar‐ │
       │                     │ ray of strings)               │
       ├─────────────────────┼───────────────────────────────┤
       │.Config ...          │ Structure with config info    │
       ├─────────────────────┼───────────────────────────────┤
       │.ConmonPidFile       │ Path to file containing  con‐ │
       │                     │ mon pid (string)              │
       ├─────────────────────┼───────────────────────────────┤
       │.Created             │ Container    creation    time │
       │                     │ (string, ISO3601)             │
       ├─────────────────────┼───────────────────────────────┤
       │.Dependencies        │ Dependencies    (array     of │
       │                     │ strings)                      │

[TomSweeneyRedHat]

@cblecker sorry, had another suggestion for consideration. Regardless if you take it or not, LGTM.

[TomSweeneyRedHat]

LGTM
and happy green test buttons!

[mtrmac]

@mtrmac The goal here was to have output filters from podman images inspect --format work the same as skopeo inspect --format.

Well, that’s not actually what this PR does; Podman uses pkg/report.New, not pkg/report.NewTemplate. The two differ in handling of \n directives, and tabulating output using tabwriter.

Now… as it happens, the net effect of this PR is actually that pkg/report.NewTemplate().Parse()… is equivalent to not using NewTemplate at all (because existing callers of printTmpl both use NormalizeFormat first); it ends up having the same result as a template.New(…).Funcs(report.DefaultFuncs).Parse(). And the latter is clearly backwards-compatible; so I propose doing just that.

Alternatively, Skopeo could be moved to use the pkg.Formatter from pkg/report.New in full; AFAICS that would be an incompatible change in principle, but not in practice; it differs for

• formats that start with {{range .}}, which don’t work with the data we print anyway
• for formats with \t, which are currently completely broken.

The version in this PR right now, using a function from pkg/report that nothing else uses (any more, AFAICS), is not very attractive.

[mtrmac]

• for formats with \t, which are currently completely broken.

#1921, for the record.

[mtrmac]

… actually, looking at the history, as of the original #1070 Skopeo’s inspect format handling really was essentially equivalent to Podman’s; it’s Podman that was updated, leaving Skopeo behind.

So updating to catch up, and to use report.New, might well make sense. (In a sense, we got very lucky with the #1921 bug, in that there is no compatibility tradeoff.)

[mtrmac]

looking at the history, as of the original #1070 Skopeo’s inspect format handling really was essentially equivalent to Podman’s

https://github.com/containers/podman/blob/v2.2.0/cmd/podman/inspect/inspect.go , so that I don’t need to look that up again

[cblecker]

Thank you for the detailed review, @mtrmac. That totally makes sense.

It looks like containers/podman#15673 was where they made changes to how reports were done in podman, and I pulled those changes over. Compiled locally, and it seems to work as intended.

[mtrmac]

Thanks!

One last change, please.

[mtrmac]

LGTM. Thanks!