From 3901f385a235cfb533b3f7c73a132e2b171b0bd2 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 16 Sep 2024 11:59:26 +0200 Subject: [PATCH 1/5] cmd: new otk external: otk-resolve-ostree-commit Implement input and output types for new ostree commit resolver for otk. --- cmd/otk-resolve-ostree-commit/main.go | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 cmd/otk-resolve-ostree-commit/main.go diff --git a/cmd/otk-resolve-ostree-commit/main.go b/cmd/otk-resolve-ostree-commit/main.go new file mode 100644 index 000000000..451a74451 --- /dev/null +++ b/cmd/otk-resolve-ostree-commit/main.go @@ -0,0 +1,44 @@ +package main + +// All otk external inputs are nested under a top-level "tree" +type Tree struct { + Tree Input `json:"tree"` +} + +// Input represents the user-provided inputs that will be used to resolve an +// ostree commit ID. +type Input struct { + // URL of the repo where the commit can be fetched. + URL string `json:"url"` + + // Ref to resolve. + Ref string `json:"ref"` + + // Whether to use RHSM secrets when resolving and fetching the commit. + RHSM bool `json:"rhsm,omitempty"` +} + +// Output contains everything needed to write a manifest that requires pulling +// an ostree commit. +type Output struct { + Const OutputConst `json:"const"` +} + +type OutputConst struct { + // Ref of the commit (can be empty). + Ref string `json:"ref,omitempty"` + + // URL of the repo where the commit can be fetched. + URL string `json:"url"` + + // Secrets type to use when pulling the ostree commit content + // (e.g. org.osbuild.rhsm.consumer). + Secrets string `json:"secrets,omitempty"` + + // Checksum of the commit. + Checksum string `json:"checksum"` +} + +func main() { + +} From 6425bc2ce7f5faf2f2ee5cf40c8bb12c5e715158 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 16 Sep 2024 12:23:25 +0200 Subject: [PATCH 2/5] cmd/otk-resolve-ostree-commit: implement resolver The resolver is a simple call to ostree.Resolve() which handles resolving ostree input options to an ostree.CommitSpec. The resolver returns the input as-is when the ref is a commit ID (there is nothing to resolve). --- cmd/otk-resolve-ostree-commit/main.go | 44 ++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/cmd/otk-resolve-ostree-commit/main.go b/cmd/otk-resolve-ostree-commit/main.go index 451a74451..e874443a3 100644 --- a/cmd/otk-resolve-ostree-commit/main.go +++ b/cmd/otk-resolve-ostree-commit/main.go @@ -1,5 +1,14 @@ package main +import ( + "encoding/json" + "fmt" + "io" + "os" + + "github.com/osbuild/images/pkg/ostree" +) + // All otk external inputs are nested under a top-level "tree" type Tree struct { Tree Input `json:"tree"` @@ -39,6 +48,39 @@ type OutputConst struct { Checksum string `json:"checksum"` } -func main() { +func run(r io.Reader, w io.Writer) error { + var inputTree Tree + if err := json.NewDecoder(r).Decode(&inputTree); err != nil { + return err + } + + sourceSpec := ostree.SourceSpec(inputTree.Tree) + commitSpec, err := ostree.Resolve(sourceSpec) + if err != nil { + return fmt.Errorf("failed to resolve ostree commit: %w", err) + } + output := map[string]Output{ + "tree": { + Const: OutputConst{ + Ref: commitSpec.Ref, + URL: commitSpec.URL, + Secrets: commitSpec.Secrets, + Checksum: commitSpec.Checksum, + }, + }, + } + outputJson, err := json.MarshalIndent(output, "", " ") + if err != nil { + return fmt.Errorf("cannot marshal response: %w", err) + } + fmt.Fprintf(w, "%s\n", outputJson) + return nil +} + +func main() { + if err := run(os.Stdin, os.Stdout); err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err.Error()) + os.Exit(1) + } } From 49bac934e0bfccb4710e1512e7c896dd1ad7e235 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 16 Sep 2024 13:33:51 +0200 Subject: [PATCH 3/5] ostree: fix error check for rhsm.LoadSystemSubscriptions() call The condition would panic if an error was returned from the function because 'subs' would be nil. Also, the underlying error would be lost if it wasn't nil. Handle 'err != nil' separately to avoid dereferencing a nil 'subs' and append the underlying error. Keep the 'subs.Consume' nil check as well since the function can succeed but not attach Consumer secrets when they're not available. 'subs' should never be nil when the function succeeds. --- pkg/ostree/ostree.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/ostree/ostree.go b/pkg/ostree/ostree.go index f7f50bb1c..a6ebe155c 100644 --- a/pkg/ostree/ostree.go +++ b/pkg/ostree/ostree.go @@ -152,7 +152,10 @@ func ResolveRef(location, ref string, consumerCerts bool, subs *rhsm.Subscriptio if consumerCerts { if subs == nil { subs, err = rhsm.LoadSystemSubscriptions() - if subs.Consumer == nil || err != nil { + if err != nil { + return "", NewResolveRefError("error adding rhsm certificates when resolving ref: %s", err) + } + if subs.Consumer == nil { return "", NewResolveRefError("error adding rhsm certificates when resolving ref") } } From fe7384afb6b3dfa93e85ca8ff7a18f409ed26cf6 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 16 Sep 2024 13:43:27 +0200 Subject: [PATCH 4/5] cmd/otk-resolve-ostree-commit: add resolver test --- cmd/otk-resolve-ostree-commit/export_test.go | 5 ++ cmd/otk-resolve-ostree-commit/main_test.go | 79 ++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 cmd/otk-resolve-ostree-commit/export_test.go create mode 100644 cmd/otk-resolve-ostree-commit/main_test.go diff --git a/cmd/otk-resolve-ostree-commit/export_test.go b/cmd/otk-resolve-ostree-commit/export_test.go new file mode 100644 index 000000000..77c5f1a93 --- /dev/null +++ b/cmd/otk-resolve-ostree-commit/export_test.go @@ -0,0 +1,5 @@ +package main + +var ( + Run = run +) diff --git a/cmd/otk-resolve-ostree-commit/main_test.go b/cmd/otk-resolve-ostree-commit/main_test.go new file mode 100644 index 000000000..43bb554dc --- /dev/null +++ b/cmd/otk-resolve-ostree-commit/main_test.go @@ -0,0 +1,79 @@ +package main_test + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + resolver "github.com/osbuild/images/cmd/otk-resolve-ostree-commit" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Create a test server that responds with the commit ID that corresponds to +// the ref. +func createTestServer(refIDs map[string]string) *httptest.Server { + handler := http.NewServeMux() + handler.HandleFunc("/refs/heads/", func(w http.ResponseWriter, r *http.Request) { + reqRef := strings.TrimPrefix(r.URL.Path, "/refs/heads/") + id, ok := refIDs[reqRef] + if !ok { + http.NotFound(w, r) + return + } + fmt.Fprint(w, id) + }) + + return httptest.NewServer(handler) +} + +func TestResolver(t *testing.T) { + commitMap := map[string]string{ + "centos/9/x86_64/edge": "d04105393ca0617856b34f897842833d785522e41617e17dca2063bf97e294ef", + "fake/ref/f": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "fake/ref/9": "9999999999999999999999999999999999999999999999999999999999999999", + "test/ref/alpha": "9b1ea9a8e10dc27d4ea40545bec028ad8e360dd26d18de64b0f6217833a8443d", + "test/ref/one": "7433e1b49fb136d61dcca49ebe34e713fdbb8e29bf328fe90819628f71b86105", + } + + require := require.New(t) + assert := assert.New(t) + + repoServer := createTestServer(commitMap) + defer repoServer.Close() + + url := repoServer.URL + for ref, id := range commitMap { + inputReq, err := json.Marshal(map[string]map[string]string{ + "tree": { + "url": url, + "ref": ref, + }, + }) + require.NoError(err) + + inpBuf := bytes.NewBuffer(inputReq) + outBuf := &bytes.Buffer{} + + assert.NoError(resolver.Run(inpBuf, outBuf)) + + var output map[string]map[string]map[string]string + require.NoError(json.Unmarshal(outBuf.Bytes(), &output)) + + expOutput := map[string]map[string]map[string]string{ + "tree": { + "const": { + "url": url, + "ref": ref, + "checksum": id, + }, + }, + } + assert.Equal(expOutput, output) + } + +} From 2a34a8eec062c27e96f80add4964b30a3460c31a Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 16 Sep 2024 13:55:05 +0200 Subject: [PATCH 5/5] cmd/otk-resolve-ostree-commit: add more test variants Added tests for calling the resolver with a commit ID (see below) with and without a URL as well as error tests. Resolving by ID: When a commit ID is used in place of the ref, the resolver returns the input commit ID as both the ref and the checksum. No request goes to the server for this call. --- cmd/otk-resolve-ostree-commit/main_test.go | 130 +++++++++++++++++++-- 1 file changed, 122 insertions(+), 8 deletions(-) diff --git a/cmd/otk-resolve-ostree-commit/main_test.go b/cmd/otk-resolve-ostree-commit/main_test.go index 43bb554dc..841ff5622 100644 --- a/cmd/otk-resolve-ostree-commit/main_test.go +++ b/cmd/otk-resolve-ostree-commit/main_test.go @@ -14,6 +14,14 @@ import ( "github.com/stretchr/testify/require" ) +var commitMap = map[string]string{ + "centos/9/x86_64/edge": "d04105393ca0617856b34f897842833d785522e41617e17dca2063bf97e294ef", + "fake/ref/f": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "fake/ref/9": "9999999999999999999999999999999999999999999999999999999999999999", + "test/ref/alpha": "9b1ea9a8e10dc27d4ea40545bec028ad8e360dd26d18de64b0f6217833a8443d", + "test/ref/one": "7433e1b49fb136d61dcca49ebe34e713fdbb8e29bf328fe90819628f71b86105", +} + // Create a test server that responds with the commit ID that corresponds to // the ref. func createTestServer(refIDs map[string]string) *httptest.Server { @@ -32,14 +40,6 @@ func createTestServer(refIDs map[string]string) *httptest.Server { } func TestResolver(t *testing.T) { - commitMap := map[string]string{ - "centos/9/x86_64/edge": "d04105393ca0617856b34f897842833d785522e41617e17dca2063bf97e294ef", - "fake/ref/f": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - "fake/ref/9": "9999999999999999999999999999999999999999999999999999999999999999", - "test/ref/alpha": "9b1ea9a8e10dc27d4ea40545bec028ad8e360dd26d18de64b0f6217833a8443d", - "test/ref/one": "7433e1b49fb136d61dcca49ebe34e713fdbb8e29bf328fe90819628f71b86105", - } - require := require.New(t) assert := assert.New(t) @@ -75,5 +75,119 @@ func TestResolver(t *testing.T) { } assert.Equal(expOutput, output) } +} + +func TestResolverByID(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + for _, id := range commitMap { + inputReq, err := json.Marshal(map[string]map[string]string{ + "tree": { + "ref": id, + }, + }) + require.NoError(err) + + inpBuf := bytes.NewBuffer(inputReq) + outBuf := &bytes.Buffer{} + assert.NoError(resolver.Run(inpBuf, outBuf)) + + var output map[string]map[string]map[string]string + require.NoError(json.Unmarshal(outBuf.Bytes(), &output)) + + expOutput := map[string]map[string]map[string]string{ + "tree": { + "const": { + "ref": id, + "checksum": id, + "url": "", + }, + }, + } + assert.Equal(expOutput, output) + } +} +func TestResolverIDwithURL(t *testing.T) { + + require := require.New(t) + assert := assert.New(t) + + // the URL is not used when the ref is a commit ID, but it should be returned in the output + url := "https://doesnt-matter.example.org" + for _, id := range commitMap { + inputReq, err := json.Marshal(map[string]map[string]string{ + "tree": { + "ref": id, + "url": url, + }, + }) + require.NoError(err) + + inpBuf := bytes.NewBuffer(inputReq) + outBuf := &bytes.Buffer{} + + assert.NoError(resolver.Run(inpBuf, outBuf)) + + var output map[string]map[string]map[string]string + require.NoError(json.Unmarshal(outBuf.Bytes(), &output)) + + expOutput := map[string]map[string]map[string]string{ + "tree": { + "const": { + "ref": id, + "checksum": id, + "url": url, + }, + }, + } + assert.Equal(expOutput, output) + } +} + +func TestResolverErrors(t *testing.T) { + + repoServer := createTestServer(commitMap) + defer repoServer.Close() + + type testCase struct { + url string + ref string + errSubstring string + } + + testCases := map[string]testCase{ + "bad-ref": { + url: "doesn't matter", + ref: "---", + errSubstring: "Invalid ostree ref or commit", + }, + "ref-not-found": { + url: repoServer.URL, + ref: "good/ref/but/does-not-exist", + errSubstring: "returned status: 404 Not Found", + }, + } + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + inputReq, err := json.Marshal(map[string]map[string]string{ + "tree": { + "url": tc.url, + "ref": tc.ref, + }, + }) + require.NoError(err) + + inpBuf := bytes.NewBuffer(inputReq) + outBuf := &bytes.Buffer{} + + assert.ErrorContains(resolver.Run(inpBuf, outBuf), tc.errSubstring) + }) + } }