Repository: osbuild/osbuild-composer
Pull request #1203 (2021-02-03 20:09:56)

Clear up confusing source TOML support
This pull request includes:

- [ ] adequate testing for the new functionality or fixed issue
- [ ] adequate documentation informing people about the change
- [ ] PR contains automated tests for new functionality and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and



Patch:
diff --git a/NEWS.md b/NEWS.md
index 99917043e..2db7b5491 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,39 @@
 # OSBuild Composer - Operating System Image Composition Services
 
+## CHANGES WITH 27:
+
+  * Starting from this release, we are putting longer news posts in
+    `docs/news/*/`. We strongly encourage everyone to take a look at them!
+
+  * OSBuild Composer now ships with a new osbuild-composer-core subpackage.
+    It neither requires systemd nor a local worker, so it's perfect to use in
+    a container. Just connect a remote worker to it and you are good to go!
+
+  * The RHEL 8.4 guest image is now much closer to its original kickstart
+    definition.
+    
+  * The API for Koji now supports retrieving manifests.
+
+  * Uploaded AMIs now have a name tag, making them easier to search for in
+    the AWS console.
+    
+  * The repository now contains a `Dockerfile` and `docker-compose.yml` for
+    a quick setup of a development environment. Feel free to use it and let
+    us know if you find it useful. We strongly recommend reading `HACKING.md`
+    to learn about some caveats of this setup.
+    
+  * The bug allowing users to override a system repository in the Weldr API
+    is now fixed.
+    
+  * The project now requires Go 1.14.
+
+Contributions from: Achilleas Koutsou, Brian C. Lane, David Rheinsberg,
+                    Jacob Kozol, Lars Karlitski, Major Hayden, Ondřej Budai,
+                    Sanne Raymaekers, Tomas Hozza, Tom Gundersen
+
+— Kořenov, 2020-02-04
+
+
 ## CHANGES WITH 26:
 
   * RHEL 8.4 images got plenty of updates:
diff --git a/docs/news/unreleased/koji-manifests.md b/docs/news/27/koji-manifests.md
similarity index 100%
rename from docs/news/unreleased/koji-manifests.md
rename to docs/news/27/koji-manifests.md
diff --git a/docs/news/unreleased/rhel84-differences-fixed.md b/docs/news/27/rhel84-differences-fixed.md
similarity index 100%
rename from docs/news/unreleased/rhel84-differences-fixed.md
rename to docs/news/27/rhel84-differences-fixed.md
diff --git a/docs/news/unreleased/timedatex.md b/docs/news/27/timedatex.md
similarity index 100%
rename from docs/news/unreleased/timedatex.md
rename to docs/news/27/timedatex.md
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index 8b9764723..48f89635d 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -8,6 +8,7 @@ import (
 	errors_package "errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"math"
 	"math/big"
@@ -590,7 +591,28 @@ func DecodeSourceConfigV0(body io.Reader, contentType string) (source SourceConf
 	if contentType == "application/json" {
 		err = json.NewDecoder(body).Decode(&source)
 	} else if contentType == "text/x-toml" {
-		_, err = toml.DecodeReader(body, &source)
+		// Read all of body in case it needs to be parsed twice
+		var data []byte
+		data, err = ioutil.ReadAll(body)
+		if err != nil {
+			return source, err
+		}
+
+		// First try to parse [ID]\n...SOURCE... form
+		var srcMap map[string]SourceConfigV0
+		err = toml.Unmarshal(data, &srcMap)
+		if err != nil {
+			// Failed, parse it again without [ID] mapping
+			err = toml.Unmarshal(data, &source)
+		} else {
+			// It is possible more than 1 source could be posted. We only use the first, after sorting
+			keys := make([]string, 0, len(srcMap))
+			for k := range srcMap {
+				keys = append(keys, k)
+			}
+			sort.Strings(keys)
+			source = srcMap[keys[0]]
+		}
 	} else {
 		err = errors_package.New("blueprint must be in json or toml format")
 	}
@@ -602,7 +624,33 @@ func DecodeSourceConfigV1(body io.Reader, contentType string) (source SourceConf
 	if contentType == "application/json" {
 		err = json.NewDecoder(body).Decode(&source)
 	} else if contentType == "text/x-toml" {
-		_, err = toml.DecodeReader(body, &source)
+		// Read all of body in case it needs to be parsed twice
+		var data []byte
+		data, err = ioutil.ReadAll(body)
+		if err != nil {
+			return source, err
+		}
+
+		// First try to parse [ID]\n...SOURCE... form
+		var srcMap map[string]SourceConfigV1
+		err = toml.Unmarshal(data, &srcMap)
+		if err != nil {
+			// Failed, parse it without [ID]
+			err = toml.Unmarshal(data, &source)
+		} else {
+			// It is possible more than 1 source could be posted. We only use the first, after sorting
+			keys := make([]string, 0, len(srcMap))
+			for k := range srcMap {
+				keys = append(keys, k)
+			}
+			sort.Strings(keys)
+			source = srcMap[keys[0]]
+
+			// If no id was set, use the ID from the map
+			if len(source.ID) == 0 {
+				source.ID = keys[0]
+			}
+		}
 	} else {
 		err = errors_package.New("blueprint must be in json or toml format")
 	}
diff --git a/osbuild-composer.spec b/osbuild-composer.spec
index 87aef2ae3..ef09e55c4 100644
--- a/osbuild-composer.spec
+++ b/osbuild-composer.spec
@@ -4,7 +4,7 @@
 
 %global goipath         github.com/osbuild/osbuild-composer
 
-Version:        26
+Version:        27
 
 %gometa
 
diff --git a/tools/image-info b/tools/image-info
index 1c2a5adc7..61c84830e 100755
--- a/tools/image-info
+++ b/tools/image-info
@@ -499,9 +499,6 @@ def append_ostree_repo(report, repo):
     resolved = {r: ostree("rev-parse", r).stdout.strip() for r in refs}
     commit = resolved[refs[0]]
 
-    refs = {r: {"inputhash": ostree("show", "--print-metadata-key=rpmostree.inputhash", resolved[r]).stdout.strip("'\n")} for r in refs}
-    report["ostree"]["refs"] = refs
-
     with tempfile.TemporaryDirectory(dir="/var/tmp") as tmpdir:
         tree = os.path.join(tmpdir, "tree")
         ostree("checkout", "--force-copy", commit, tree)


Test patch:
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index 70e9546df..65eb7603a 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -980,40 +980,46 @@ func TestSourcesNew(t *testing.T) {
 	}
 }
 
-func TestSourcesNewToml(t *testing.T) {
+func TestSourcesNewTomlV0(t *testing.T) {
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")
 	require.NoError(t, err)
 	defer os.RemoveAll(tempdir)
 
-	source := `
+	sources := []string{`
 name = "fish"
 url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
 type = "yum-baseurl"
 check_ssl = false
 check_gpg = false
-`
-	req := httptest.NewRequest("POST", "/api/v0/projects/source/new", bytes.NewReader([]byte(source)))
-	req.Header.Set("Content-Type", "text/x-toml")
-	recorder := httptest.NewRecorder()
+`, `[fish]
+name = "fish"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`}
+	for _, source := range sources {
+		req := httptest.NewRequest("POST", "/api/v0/projects/source/new", bytes.NewReader([]byte(source)))
+		req.Header.Set("Content-Type", "text/x-toml")
+		recorder := httptest.NewRecorder()
 
-	api, _ := createWeldrAPI(tempdir, rpmmd_mock.BaseFixture)
-	api.ServeHTTP(recorder, req)
+		api, _ := createWeldrAPI(tempdir, rpmmd_mock.BaseFixture)
+		api.ServeHTTP(recorder, req)
 
-	r := recorder.Result()
-	require.Equal(t, http.StatusOK, r.StatusCode)
+		r := recorder.Result()
+		require.Equal(t, http.StatusOK, r.StatusCode)
 
-	test.SendHTTP(api, true, "DELETE", "/api/v0/projects/source/delete/fish", ``)
+		test.SendHTTP(api, true, "DELETE", "/api/v0/projects/source/delete/fish", ``)
+	}
 }
 
 // Empty TOML, and invalid TOML should return an error
-func TestSourcesNewWrongToml(t *testing.T) {
+func TestSourcesNewWrongTomlV0(t *testing.T) {
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")
 	require.NoError(t, err)
 	defer os.RemoveAll(tempdir)
 
 	sources := []string{``, `
-[fish]
-name = "fish"
 url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
 type = "yum-baseurl"
 check_ssl = false
@@ -1032,6 +1038,85 @@ check_gpg = false
 	}
 }
 
+// TestSourcesNewTomlV1 tests the v1 sources API with id and name
+func TestSourcesNewTomlV1(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "weldr-tests-")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	sources := []string{`
+id = "fish"
+name = "fish or cut bait"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`, `[fish]
+name = "fish or cut bait"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`, `[fish]
+id = "fish"
+name = "fish or cut bait"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`}
+	for _, source := range sources {
+		req := httptest.NewRequest("POST", "/api/v1/projects/source/new", bytes.NewReader([]byte(source)))
+		req.Header.Set("Content-Type", "text/x-toml")
+		recorder := httptest.NewRecorder()
+
+		api, _ := createWeldrAPI(tempdir, rpmmd_mock.BaseFixture)
+		api.ServeHTTP(recorder, req)
+
+		r := recorder.Result()
+		require.Equal(t, http.StatusOK, r.StatusCode)
+
+		test.SendHTTP(api, true, "DELETE", "/api/v1/projects/source/delete/fish", ``)
+	}
+}
+
+// TestSourcesNewWrongTomlV1 Tests that Empty TOML, and invalid TOML should return an error
+func TestSourcesNewWrongTomlV1(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "weldr-tests-")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	sources := []string{``, `
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`, `
+[fish]
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`, `
+id = "fish"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`}
+	for _, source := range sources {
+		req := httptest.NewRequest("POST", "/api/v1/projects/source/new", bytes.NewReader([]byte(source)))
+		req.Header.Set("Content-Type", "text/x-toml")
+		recorder := httptest.NewRecorder()
+
+		api, _ := createWeldrAPI(tempdir, rpmmd_mock.BaseFixture)
+		api.ServeHTTP(recorder, req)
+
+		r := recorder.Result()
+		require.Equal(t, http.StatusBadRequest, r.StatusCode)
+	}
+}
+
 func TestSourcesInfo(t *testing.T) {
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")
 	require.NoError(t, err)
diff --git a/test/data/manifests/fedora_32-x86_64-fedora_iot_commit-boot.json b/test/data/manifests/fedora_32-x86_64-fedora_iot_commit-boot.json
index f6bc7adb0..693bcfe5d 100644
--- a/test/data/manifests/fedora_32-x86_64-fedora_iot_commit-boot.json
+++ b/test/data/manifests/fedora_32-x86_64-fedora_iot_commit-boot.json
@@ -10213,11 +10213,9 @@
       "VERSION_ID": "32"
     },
     "ostree": {
-      "refs": {
-        "fedora/32/x86_64/iot": {
-          "inputhash": "e7a265e665ce2765c90c1a51ba187202e8b77b4e0aadddfc545189171cb9c31f"
-        }
-      },
+      "refs": [
+        "fedora/32/x86_64/iot"
+      ],
       "repo": {
         "core.mode": "archive-z2"
       }
@@ -10742,4 +10740,4 @@
     ],
     "type": "ostree/commit"
   }
-}
\ No newline at end of file
+}
diff --git a/test/data/manifests/fedora_33-x86_64-fedora_iot_commit-boot.json b/test/data/manifests/fedora_33-x86_64-fedora_iot_commit-boot.json
index ce3366bac..ced2ade52 100644
--- a/test/data/manifests/fedora_33-x86_64-fedora_iot_commit-boot.json
+++ b/test/data/manifests/fedora_33-x86_64-fedora_iot_commit-boot.json
@@ -10659,11 +10659,9 @@
       "VERSION_ID": "33"
     },
     "ostree": {
-      "refs": {
-        "fedora/33/x86_64/iot": {
-          "inputhash": "af23373725489f462ab563895f7ffff450504d0057b8e4569244e9884c3ceb71"
-        }
-      },
+      "refs": [
+        "fedora/33/x86_64/iot"
+      ],
       "repo": {
         "core.mode": "archive-z2"
       }
@@ -11211,4 +11209,4 @@
     "timezone": "UTC",
     "type": "ostree/commit"
   }
-}
\ No newline at end of file
+}
diff --git a/test/data/manifests/rhel_8-aarch64-rhel_edge_commit-boot.json b/test/data/manifests/rhel_8-aarch64-rhel_edge_commit-boot.json
index 9db9cf82e..372b26ffa 100644
--- a/test/data/manifests/rhel_8-aarch64-rhel_edge_commit-boot.json
+++ b/test/data/manifests/rhel_8-aarch64-rhel_edge_commit-boot.json
@@ -8276,11 +8276,9 @@
       "VERSION_ID": "8.3"
     },
     "ostree": {
-      "refs": {
-        "rhel/8/aarch64/edge": {
-          "inputhash": "e58e63f14f7edbb5add61ef8f65bf84f5934cdb0e4119caeca5a5e04461f65d3"
-        }
-      },
+      "refs": [
+        "rhel/8/aarch64/edge"
+      ],
       "repo": {
         "core.mode": "archive-z2"
       }
@@ -8752,4 +8750,4 @@
     "timezone": "UTC",
     "type": "ostree/commit"
   }
-}
\ No newline at end of file
+}
diff --git a/test/data/manifests/rhel_8-x86_64-rhel_edge_commit-boot.json b/test/data/manifests/rhel_8-x86_64-rhel_edge_commit-boot.json
index c18a3a6f7..8dec00f16 100644
--- a/test/data/manifests/rhel_8-x86_64-rhel_edge_commit-boot.json
+++ b/test/data/manifests/rhel_8-x86_64-rhel_edge_commit-boot.json
@@ -8561,11 +8561,9 @@
       "VERSION_ID": "8.3"
     },
     "ostree": {
-      "refs": {
-        "rhel/8/x86_64/edge": {
-          "inputhash": "e255d15d889add2e2a1e8dc4fad9d73b61d5e2f2608f0713cf7f99a229cdaa49"
-        }
-      },
+      "refs": [
+        "rhel/8/x86_64/edge"
+      ],
       "repo": {
         "core.mode": "archive-z2"
       }
@@ -9053,4 +9051,4 @@
     "timezone": "UTC",
     "type": "ostree/commit"
   }
-}
\ No newline at end of file
+}