Repository: osbuild/osbuild-composer (go)

Issue:
Compose upload to AWS hangs when snapshot import fails
I started a compose with upload to AWS enabled, and the snapshot went into `deleted` status:

```
            "ImportTaskId": "import-snap-0bd6fdf1d41b31bcb",
            "SnapshotTaskDetail": {
                "DiskImageSize": 0.0,
                "Format": "VHDX",
                "Status": "deleted",
                "StatusMessage": "ClientError: Disk validation failed [Unsupported specified file format]",
                "UserBucket": {
                    "S3Bucket": "imagebuilder-jenkins-testing-use2",
                    "S3Key": "ansible-osbuild-smoke-test-030af12e-431a-527b-be68-3d1de91352a1"
                }
            },
            "Tags": []
        },
```

However, osbuild-composer still leaves the compose marked as `RUNNING` even though this snapshot will never fully import.

Pull request description:
upload/aws: fail ImportSnapshot when the snapshot is deleted
When ImportSnapshot fails, it goes into the deleted state. However, the code
didn't consider this state as a failure. This commit fixes that.

Fixes #706

Patch:
diff --git a/internal/client/source.go b/internal/client/source.go
index c004c2692..03cac0edf 100644
--- a/internal/client/source.go
+++ b/internal/client/source.go
@@ -25,13 +25,41 @@ func ListSourcesV0(socket *http.Client) ([]string, *APIResponse, error) {
 	return list.Sources, nil, nil
 }
 
+// ListSourcesV1 returns a list of source ids
+func ListSourcesV1(socket *http.Client) ([]string, *APIResponse, error) {
+	body, resp, err := GetRaw(socket, "GET", "/api/v1/projects/source/list")
+	if resp != nil || err != nil {
+		return nil, resp, err
+	}
+	var list weldr.SourceListV1
+	err = json.Unmarshal(body, &list)
+	if err != nil {
+		return nil, nil, err
+	}
+	return list.Sources, nil, nil
+}
+
 // GetSourceInfoV0 returns detailed information on the named sources
 func GetSourceInfoV0(socket *http.Client, sourceNames string) (map[string]weldr.SourceConfigV0, *APIResponse, error) {
 	body, resp, err := GetRaw(socket, "GET", "/api/v0/projects/source/info/"+sourceNames)
 	if resp != nil || err != nil {
 		return nil, resp, err
 	}
-	var info weldr.SourceInfoV0
+	var info weldr.SourceInfoResponseV0
+	err = json.Unmarshal(body, &info)
+	if err != nil {
+		return nil, nil, err
+	}
+	return info.Sources, nil, nil
+}
+
+// GetSourceInfoV1 returns detailed information on the named sources
+func GetSourceInfoV1(socket *http.Client, sourceNames string) (map[string]weldr.SourceConfigV1, *APIResponse, error) {
+	body, resp, err := GetRaw(socket, "GET", "/api/v1/projects/source/info/"+sourceNames)
+	if resp != nil || err != nil {
+		return nil, resp, err
+	}
+	var info weldr.SourceInfoResponseV1
 	err = json.Unmarshal(body, &info)
 	if err != nil {
 		return nil, nil, err
@@ -49,6 +77,16 @@ func PostJSONSourceV0(socket *http.Client, source string) (*APIResponse, error)
 	return NewAPIResponse(body)
 }
 
+// PostJSONSourceV1 sends a JSON source string to the API
+// and returns an APIResponse
+func PostJSONSourceV1(socket *http.Client, source string) (*APIResponse, error) {
+	body, resp, err := PostJSON(socket, "/api/v1/projects/source/new", source)
+	if resp != nil || err != nil {
+		return resp, err
+	}
+	return NewAPIResponse(body)
+}
+
 // PostTOMLSourceV0 sends a TOML source string to the API
 // and returns an APIResponse
 func PostTOMLSourceV0(socket *http.Client, source string) (*APIResponse, error) {
@@ -59,6 +97,16 @@ func PostTOMLSourceV0(socket *http.Client, source string) (*APIResponse, error)
 	return NewAPIResponse(body)
 }
 
+// PostTOMLSourceV1 sends a TOML source string to the API
+// and returns an APIResponse
+func PostTOMLSourceV1(socket *http.Client, source string) (*APIResponse, error) {
+	body, resp, err := PostTOML(socket, "/api/v1/projects/source/new", source)
+	if resp != nil || err != nil {
+		return resp, err
+	}
+	return NewAPIResponse(body)
+}
+
 // DeleteSourceV0 deletes the named source and returns an APIResponse
 func DeleteSourceV0(socket *http.Client, sourceName string) (*APIResponse, error) {
 	body, resp, err := DeleteRaw(socket, "/api/v0/projects/source/delete/"+sourceName)
@@ -67,3 +115,12 @@ func DeleteSourceV0(socket *http.Client, sourceName string) (*APIResponse, error
 	}
 	return NewAPIResponse(body)
 }
+
+// DeleteSourceV1 deletes the named source and returns an APIResponse
+func DeleteSourceV1(socket *http.Client, sourceName string) (*APIResponse, error) {
+	body, resp, err := DeleteRaw(socket, "/api/v1/projects/source/delete/"+sourceName)
+	if resp != nil || err != nil {
+		return resp, err
+	}
+	return NewAPIResponse(body)
+}
diff --git a/internal/store/store.go b/internal/store/store.go
index c2a473cbd..7e3c7406f 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -407,23 +407,55 @@ func (s *Store) DeleteCompose(id uuid.UUID) error {
 	})
 }
 
-func (s *Store) PushSource(source SourceConfig) {
+// PushSource stores a SourceConfig in store.Sources
+func (s *Store) PushSource(key string, source SourceConfig) {
 	// FIXME: handle or comment this possible error
 	_ = s.change(func() error {
-		s.sources[source.Name] = source
+		s.sources[key] = source
 		return nil
 	})
 }
 
-func (s *Store) DeleteSource(name string) {
+// DeleteSourceByName removes a SourceConfig from store.Sources using the .Name field
+func (s *Store) DeleteSourceByName(name string) {
 	// FIXME: handle or comment this possible error
 	_ = s.change(func() error {
-		delete(s.sources, name)
+		for key := range s.sources {
+			if s.sources[key].Name == name {
+				delete(s.sources, key)
+				return nil
+			}
+		}
+		return nil
+	})
+}
+
+// DeleteSourceByID removes a SourceConfig from store.Sources using the ID
+func (s *Store) DeleteSourceByID(key string) {
+	// FIXME: handle or comment this possible error
+	_ = s.change(func() error {
+		delete(s.sources, key)
 		return nil
 	})
 }
 
-func (s *Store) ListSources() []string {
+// ListSourcesByName returns the repo source names
+// Name is different than Id, it can be a full description of the repo
+func (s *Store) ListSourcesByName() []string {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	names := make([]string, 0, len(s.sources))
+	for _, source := range s.sources {
+		names = append(names, source.Name)
+	}
+	sort.Strings(names)
+
+	return names
+}
+
+// ListSourcesById returns the repo source id
+// Id is a short identifier for the repo, not a full name description
+func (s *Store) ListSourcesById() []string {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 	names := make([]string, 0, len(s.sources))
@@ -446,7 +478,22 @@ func (s *Store) GetSource(name string) *SourceConfig {
 	return &source
 }
 
-func (s *Store) GetAllSources() map[string]SourceConfig {
+// GetAllSourcesByName returns the sources using the repo name as the key
+func (s *Store) GetAllSourcesByName() map[string]SourceConfig {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	sources := make(map[string]SourceConfig)
+
+	for _, v := range s.sources {
+		sources[v.Name] = v
+	}
+
+	return sources
+}
+
+// GetAllSourcesByID returns the sources using the repo id as the key
+func (s *Store) GetAllSourcesByID() map[string]SourceConfig {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 
@@ -481,10 +528,10 @@ func NewSourceConfig(repo rpmmd.RepoConfig, system bool) SourceConfig {
 	return sc
 }
 
-func (s *SourceConfig) RepoConfig() rpmmd.RepoConfig {
+func (s *SourceConfig) RepoConfig(name string) rpmmd.RepoConfig {
 	var repo rpmmd.RepoConfig
 
-	repo.Name = s.Name
+	repo.Name = name
 	repo.IgnoreSSL = !s.CheckSSL
 
 	if s.Type == "yum-baseurl" {
diff --git a/internal/upload/awsupload/awsupload.go b/internal/upload/awsupload/awsupload.go
index 60b621ce0..5c457dc0d 100644
--- a/internal/upload/awsupload/awsupload.go
+++ b/internal/upload/awsupload/awsupload.go
@@ -1,6 +1,7 @@
 package awsupload
 
 import (
+	"log"
 	"os"
 	"time"
 
@@ -45,6 +46,7 @@ func (a *AWS) Upload(filename, bucket, key string) (*s3manager.UploadOutput, err
 		return nil, err
 	}
 
+	log.Printf("[AWS] Uploading image to S3: %s/%s", bucket, key)
 	return a.uploader.Upload(
 		&s3manager.UploadInput{
 			Bucket: aws.String(bucket),
@@ -87,6 +89,11 @@ func WaitUntilImportSnapshotTaskCompletedWithContext(c *ec2.EC2, ctx aws.Context
 				Matcher: request.PathAllWaiterMatch, Argument: "ImportSnapshotTasks[].SnapshotTaskDetail.Status",
 				Expected: "completed",
 			},
+			{
+				State:   request.FailureWaiterState,
+				Matcher: request.PathAllWaiterMatch, Argument: "ImportSnapshotTasks[].SnapshotTaskDetail.Status",
+				Expected: "deleted",
+			},
 		},
 		Logger: c.Config.Logger,
 		NewRequest: func(opts []request.Option) (*request.Request, error) {
@@ -107,6 +114,7 @@ func WaitUntilImportSnapshotTaskCompletedWithContext(c *ec2.EC2, ctx aws.Context
 }
 
 func (a *AWS) Register(name, bucket, key string) (*string, error) {
+	log.Printf("[AWS] Importing snapshot from image: %s/%s", bucket, key)
 	importTaskOutput, err := a.importer.ImportSnapshot(
 		&ec2.ImportSnapshotInput{
 			DiskContainer: &ec2.SnapshotDiskContainer{
@@ -121,6 +129,7 @@ func (a *AWS) Register(name, bucket, key string) (*string, error) {
 		return nil, err
 	}
 
+	log.Printf("[AWS] Waiting for snapshot to finish importing: %v", importTaskOutput.ImportTaskId)
 	err = WaitUntilImportSnapshotTaskCompleted(
 		a.importer,
 		&ec2.DescribeImportSnapshotTasksInput{
@@ -134,6 +143,7 @@ func (a *AWS) Register(name, bucket, key string) (*string, error) {
 	}
 
 	// we no longer need the object in s3, let's just delete it
+	log.Printf("[AWS] Deleting image from S3: %s/%s", bucket, key)
 	_, err = a.s3.DeleteObject(&s3.DeleteObjectInput{
 		Bucket: aws.String(bucket),
 		Key:    aws.String(key),
@@ -155,6 +165,7 @@ func (a *AWS) Register(name, bucket, key string) (*string, error) {
 
 	snapshotId := importOutput.ImportSnapshotTasks[0].SnapshotTaskDetail.SnapshotId
 
+	log.Printf("[AWS] Registering AMI from imported snapshot: %v", snapshotId)
 	registerOutput, err := a.importer.RegisterImage(
 		&ec2.RegisterImageInput{
 			Architecture:       aws.String("x86_64"),
@@ -176,5 +187,6 @@ func (a *AWS) Register(name, bucket, key string) (*string, error) {
 		return nil, err
 	}
 
+	log.Printf("[AWS] 🎉 AMI registered: %v", registerOutput.ImageId)
 	return registerOutput.ImageId, nil
 }
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index 4517adb74..0ab5f9bb3 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -48,6 +48,15 @@ type API struct {
 	compatOutputDir string
 }
 
+// systemRepoIDs returns a list of the system repos
+// NOTE: The system repos have no concept of id vs. name so the id is returned
+func (api *API) systemRepoNames() (names []string) {
+	for _, repo := range api.repos {
+		names = append(names, repo.Name)
+	}
+	return names
+}
+
 var ValidBlueprintName = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
 
 func New(rpmmd rpmmd.RPMMD, arch distro.Arch, distro distro.Distro, repos []rpmmd.RepoConfig, logger *log.Logger, store *store.Store, workers *worker.Server, compatOutputDir string) *API {
@@ -348,11 +357,15 @@ func (api *API) sourceListHandler(writer http.ResponseWriter, request *http.Requ
 		Sources []string `json:"sources"`
 	}
 
-	names := api.store.ListSources()
-
-	for _, repo := range api.repos {
-		names = append(names, repo.Name)
+	// The v0 API used the repo Name, a descriptive string, as the key
+	// In the v1 API this was changed to separate the Name and the Id (a short identifier)
+	var names []string
+	if isRequestVersionAtLeast(params, 1) {
+		names = api.store.ListSourcesById()
+	} else {
+		names = api.store.ListSourcesByName()
 	}
+	names = append(names, api.systemRepoNames()...)
 
 	err := json.NewEncoder(writer).Encode(reply{
 		Sources: names,
@@ -373,18 +386,9 @@ func (api *API) sourceEmptyInfoHandler(writer http.ResponseWriter, request *http
 	statusResponseError(writer, http.StatusNotFound, errors)
 }
 
-func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
-	if !verifyRequestVersion(writer, params, 0) { // TODO: version 1 API
-		return
-	}
-
-	// weldr uses a slightly different format than dnf to store repository
-	// configuration
-	type reply struct {
-		Sources map[string]store.SourceConfig `json:"sources"`
-		Errors  []responseError               `json:"errors"`
-	}
-
+// getSourceConfigs retrieves the list of sources from the system repos an store
+// Returning a list of store.SourceConfig entries indexed by the id of the source
+func (api *API) getSourceConfigs(params httprouter.Params) (map[string]store.SourceConfig, []responseError) {
 	names := params.ByName("sources")
 
 	sources := map[string]store.SourceConfig{}
@@ -392,7 +396,7 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 
 	// if names is "*" we want all sources
 	if names == "*" {
-		sources = api.store.GetAllSources()
+		sources = api.store.GetAllSourcesByID()
 		for _, repo := range api.repos {
 			sources[repo.Name] = store.NewSourceConfig(repo, true)
 		}
@@ -412,7 +416,7 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 			}
 			// check if the source is in the store
 			if source := api.store.GetSource(name); source != nil {
-				sources[source.Name] = *source
+				sources[name] = *source
 			} else {
 				error := responseError{
 					ID:  "UnknownSource",
@@ -423,6 +427,37 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 		}
 	}
 
+	return sources, errors
+}
+
+// sourceInfoHandler routes the call to the correct API version handler
+func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	versionString := params.ByName("version")
+	version, err := strconv.ParseUint(versionString, 10, 0)
+	if err != nil {
+		notFoundHandler(writer, nil)
+		return
+	}
+	switch version {
+	case 0:
+		api.sourceInfoHandlerV0(writer, request, params)
+	case 1:
+		api.sourceInfoHandlerV1(writer, request, params)
+	default:
+		notFoundHandler(writer, nil)
+	}
+}
+
+// sourceInfoHandlerV0 handles the API v0 response
+func (api *API) sourceInfoHandlerV0(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	sources, errors := api.getSourceConfigs(params)
+
+	// V0 responses use the source name as the key
+	v0Sources := make(map[string]SourceConfigV0, len(sources))
+	for _, s := range sources {
+		v0Sources[s.Name] = NewSourceConfigV0(s)
+	}
+
 	q, err := url.ParseQuery(request.URL.RawQuery)
 	if err != nil {
 		errors := responseError{
@@ -435,8 +470,8 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 
 	format := q.Get("format")
 	if format == "json" || format == "" {
-		err := json.NewEncoder(writer).Encode(reply{
-			Sources: sources,
+		err := json.NewEncoder(writer).Encode(SourceInfoResponseV0{
+			Sources: v0Sources,
 			Errors:  errors,
 		})
 		common.PanicOnError(err)
@@ -451,8 +486,77 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 			Msg: fmt.Sprintf("invalid format parameter: %s", format),
 		}
 		statusResponseError(writer, http.StatusBadRequest, errors)
+	}
+}
+
+// sourceInfoHandlerV1 handles the API v0 response
+func (api *API) sourceInfoHandlerV1(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	sources, errors := api.getSourceConfigs(params)
+
+	// V1 responses use the source id as the key
+	v1Sources := make(map[string]SourceConfigV1, len(sources))
+	for id, s := range sources {
+		v1Sources[id] = NewSourceConfigV1(id, s)
+	}
+
+	q, err := url.ParseQuery(request.URL.RawQuery)
+	if err != nil {
+		errors := responseError{
+			ID:  "InvalidChars",
+			Msg: fmt.Sprintf("invalid query string: %v", err),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
 		return
 	}
+
+	format := q.Get("format")
+	if format == "json" || format == "" {
+		err := json.NewEncoder(writer).Encode(SourceInfoResponseV1{
+			Sources: v1Sources,
+			Errors:  errors,
+		})
+		common.PanicOnError(err)
+	} else if format == "toml" {
+		encoder := toml.NewEncoder(writer)
+		encoder.Indent = ""
+		err := encoder.Encode(sources)
+		common.PanicOnError(err)
+	} else {
+		errors := responseError{
+			ID:  "InvalidChars",
+			Msg: fmt.Sprintf("invalid format parameter: %s", format),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+	}
+}
+
+// DecodeSourceConfigV0 parses a request.Body into a SourceConfigV0
+func DecodeSourceConfigV0(body io.Reader, contentType string) (source SourceConfigV0, err error) {
+	if contentType == "application/json" {
+		err = json.NewDecoder(body).Decode(&source)
+	} else if contentType == "text/x-toml" {
+		_, err = toml.DecodeReader(body, &source)
+	} else {
+		err = errors_package.New("blueprint must be in json or toml format")
+	}
+	return source, err
+}
+
+// DecodeSourceConfigV1 parses a request.Body into a SourceConfigV1
+func DecodeSourceConfigV1(body io.Reader, contentType string) (source SourceConfigV1, err error) {
+	if contentType == "application/json" {
+		err = json.NewDecoder(body).Decode(&source)
+	} else if contentType == "text/x-toml" {
+		_, err = toml.DecodeReader(body, &source)
+	} else {
+		err = errors_package.New("blueprint must be in json or toml format")
+	}
+
+	if err == nil && len(source.GetKey()) == 0 {
+		err = errors_package.New("'id' field is missing from request")
+	}
+
+	return source, err
 }
 
 func (api *API) sourceNewHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
@@ -479,22 +583,20 @@ func (api *API) sourceNewHandler(writer http.ResponseWriter, request *http.Reque
 		return
 	}
 
-	var source SourceConfigV0
+	var source SourceConfig
 	var err error
-	if contentType[0] == "application/json" {
-		err = json.NewDecoder(request.Body).Decode(&source)
-	} else if contentType[0] == "text/x-toml" {
-		_, err = toml.DecodeReader(request.Body, &source)
+	if isRequestVersionAtLeast(params, 1) {
+		source, err = DecodeSourceConfigV1(request.Body, contentType[0])
 	} else {
-		err = errors_package.New("blueprint must be in json or toml format")
+		source, err = DecodeSourceConfigV0(request.Body, contentType[0])
 	}
 
 	// Basic check of the source, should at least have a name and type
 	if err == nil {
-		if len(source.Name) == 0 {
-			err = errors_package.New("'name' field is missing from API v0 request")
-		} else if len(source.Type) == 0 {
-			err = errors_package.New("'type' field is missing from API v0 request")
+		if len(source.GetName()) == 0 {
+			err = errors_package.New("'name' field is missing from request")
+		} else if len(source.GetType()) == 0 {
+			err = errors_package.New("'type' field is missing from request")
 		}
 	}
 
@@ -507,7 +609,7 @@ func (api *API) sourceNewHandler(writer http.ResponseWriter, request *http.Reque
 		return
 	}
 
-	api.store.PushSource(source.SourceConfig())
+	api.store.PushSource(source.GetKey(), source.SourceConfig())
 
 	statusResponseOK(writer)
 }
@@ -529,8 +631,24 @@ func (api *API) sourceDeleteHandler(writer http.ResponseWriter, request *http.Re
 		return
 	}
 
-	// remove leading / from first name
-	api.store.DeleteSource(name[0][1:])
+	// Check for system repos and return an error
+	for _, id := range api.systemRepoNames() {
+		if id == name[0][1:] {
+			errors := responseError{
+				ID:  "SystemSource",
+				Msg: id + " is a system source, it cannot be deleted.",
+			}
+			statusResponseError(writer, http.StatusBadRequest, errors)
+			return
+		}
+	}
+
+	// Only delete the first name, which will have a / at the start because of the /*source route
+	if isRequestVersionAtLeast(params, 1) {
+		api.store.DeleteSourceByID(name[0][1:])
+	} else {
+		api.store.DeleteSourceByName(name[0][1:])
+	}
 
 	statusResponseOK(writer)
 }
@@ -2357,8 +2475,8 @@ func getPkgNameGlob(pkg blueprint.Package) string {
 // Returns all configured repositories (base + sources) as rpmmd.RepoConfig
 func (api *API) allRepositories() []rpmmd.RepoConfig {
 	repos := append([]rpmmd.RepoConfig{}, api.repos...)
-	for _, source := range api.store.GetAllSources() {
-		repos = append(repos, source.RepoConfig())
+	for id, source := range api.store.GetAllSourcesByID() {
+		repos = append(repos, source.RepoConfig(id))
 	}
 	return repos
 }
diff --git a/internal/weldr/json.go b/internal/weldr/json.go
index 81d7511fa..4e1bb6dd9 100644
--- a/internal/weldr/json.go
+++ b/internal/weldr/json.go
@@ -85,6 +85,11 @@ type SourceListV0 struct {
 	Sources []string `json:"sources"`
 }
 
+// SourceListV1 is the response to /source/list request
+type SourceListV1 struct {
+	Sources []string `json:"sources"`
+}
+
 // SourceInfoV0 is the response to a /source/info request
 type SourceInfoV0 struct {
 	Sources map[string]SourceConfigV0 `json:"sources"`
@@ -101,6 +106,29 @@ func (s *SourceInfoV0) SourceConfig(sourceName string) (ssc store.SourceConfig,
 	return si.SourceConfig(), true
 }
 
+// SourceConfig interface defines the common functions needed to query the SourceConfigV0/V1 structs
+type SourceConfig interface {
+	GetKey() string
+	GetName() string
+	GetType() string
+	SourceConfig() store.SourceConfig
+}
+
+// NewSourceConfigV0 converts a store.SourceConfig to a SourceConfigV0
+// The store does not support proxy and gpgkey_urls
+func NewSourceConfigV0(s store.SourceConfig) SourceConfigV0 {
+	var sc SourceConfigV0
+
+	sc.Name = s.Name
+	sc.Type = s.Type
+	sc.URL = s.URL
+	sc.CheckGPG = s.CheckGPG
+	sc.CheckSSL = s.CheckSSL
+	sc.System = s.System
+
+	return sc
+}
+
 // SourceConfigV0 holds the source repository information
 type SourceConfigV0 struct {
 	Name     string   `json:"name" toml:"name"`
@@ -109,13 +137,28 @@ type SourceConfigV0 struct {
 	CheckGPG bool     `json:"check_gpg" toml:"check_gpg"`
 	CheckSSL bool     `json:"check_ssl" toml:"check_ssl"`
 	System   bool     `json:"system" toml:"system"`
-	Proxy    string   `json:"proxy" toml:"proxy"`
-	GPGUrls  []string `json:"gpgkey_urls" toml:"gpgkey_urls"`
+	Proxy    string   `json:"proxy,omitempty" toml:"proxy,omitempty"`
+	GPGUrls  []string `json:"gpgkey_urls,omitempty" toml:"gpgkey_urls,omitempty"`
+}
+
+// Key return the key, .Name in this case
+func (s SourceConfigV0) GetKey() string {
+	return s.Name
+}
+
+// Name return the .Name field
+func (s SourceConfigV0) GetName() string {
+	return s.Name
+}
+
+// Type return the .Type field
+func (s SourceConfigV0) GetType() string {
+	return s.Type
 }
 
 // SourceConfig returns a SourceConfig struct populated with the supported variables
 // The store does not support proxy and gpgkey_urls
-func (s *SourceConfigV0) SourceConfig() (ssc store.SourceConfig) {
+func (s SourceConfigV0) SourceConfig() (ssc store.SourceConfig) {
 	ssc.Name = s.Name
 	ssc.Type = s.Type
 	ssc.URL = s.URL
@@ -125,6 +168,74 @@ func (s *SourceConfigV0) SourceConfig() (ssc store.SourceConfig) {
 	return ssc
 }
 
+// SourceInfoResponseV0
+type SourceInfoResponseV0 struct {
+	Sources map[string]SourceConfigV0 `json:"sources"`
+	Errors  []responseError           `json:"errors"`
+}
+
+// NewSourceConfigV1 converts a store.SourceConfig to a SourceConfigV1
+// The store does not support proxy and gpgkey_urls
+func NewSourceConfigV1(id string, s store.SourceConfig) SourceConfigV1 {
+	var sc SourceConfigV1
+
+	sc.ID = id
+	sc.Name = s.Name
+	sc.Type = s.Type
+	sc.URL = s.URL
+	sc.CheckGPG = s.CheckGPG
+	sc.CheckSSL = s.CheckSSL
+	sc.System = s.System
+
+	return sc
+}
+
+// SourceConfigV1 holds the source repository information
+type SourceConfigV1 struct {
+	ID       string   `json:"id" toml:"id"`
+	Name     string   `json:"name" toml:"name"`
+	Type     string   `json:"type" toml:"type"`
+	URL      string   `json:"url" toml:"url"`
+	CheckGPG bool     `json:"check_gpg" toml:"check_gpg"`
+	CheckSSL bool     `json:"check_ssl" toml:"check_ssl"`
+	System   bool     `json:"system" toml:"system"`
+	Proxy    string   `json:"proxy,omitempty" toml:"proxy,omitempty"`
+	GPGUrls  []string `json:"gpgkey_urls,omitempty" toml:"gpgkey_urls,omitempty"`
+}
+
+// Key returns the key, .ID in this case
+func (s SourceConfigV1) GetKey() string {
+	return s.ID
+}
+
+// Name return the .Name field
+func (s SourceConfigV1) GetName() string {
+	return s.Name
+}
+
+// Type return the .Type field
+func (s SourceConfigV1) GetType() string {
+	return s.Type
+}
+
+// SourceConfig returns a SourceConfig struct populated with the supported variables
+// The store does not support proxy and gpgkey_urls
+func (s SourceConfigV1) SourceConfig() (ssc store.SourceConfig) {
+	ssc.Name = s.Name
+	ssc.Type = s.Type
+	ssc.URL = s.URL
+	ssc.CheckGPG = s.CheckGPG
+	ssc.CheckSSL = s.CheckSSL
+
+	return ssc
+}
+
+// SourceInfoResponseV1
+type SourceInfoResponseV1 struct {
+	Sources map[string]SourceConfigV1 `json:"sources"`
+	Errors  []responseError           `json:"errors"`
+}
+
 // ProjectsListV0 is the response to /projects/list request
 type ProjectsListV0 struct {
 	Total    uint                `json:"total"`


Test patch:
diff --git a/cmd/osbuild-tests/main_test.go b/cmd/osbuild-tests/main_test.go
index 440bef5f8..e53144c4d 100644
--- a/cmd/osbuild-tests/main_test.go
+++ b/cmd/osbuild-tests/main_test.go
@@ -112,7 +112,8 @@ func TestSourcesCommands(t *testing.T) {
 	require.NoErrorf(t, err, "Could not create temporary file: %v", err)
 	defer os.Remove(sources_toml.Name())
 
-	_, err = sources_toml.Write([]byte(`name = "osbuild-test-addon-source"
+	_, err = sources_toml.Write([]byte(`id = "osbuild-test-addon-source"
+name = "Testing sources add command"
 url = "file://REPO-PATH"
 type = "yum-baseurl"
 proxy = "https://proxy-url/"
diff --git a/internal/client/source_test.go b/internal/client/source_test.go
index a38d78eaa..8d26101ad 100644
--- a/internal/client/source_test.go
+++ b/internal/client/source_test.go
@@ -37,14 +37,21 @@ func TestPOSTJSONSourceV0(t *testing.T) {
 	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
 }
 
-// POST an empty JSON source
+// POST an empty JSON source using V0 API
 func TestPOSTEmptyJSONSourceV0(t *testing.T) {
 	resp, err := PostJSONSourceV0(testState.socket, "")
 	require.NoError(t, err, "POST source failed with a client error")
 	require.False(t, resp.Status, "did not return an error")
 }
 
-// POST an invalid JSON source
+// POST an empty JSON source using V1 API
+func TestPOSTEmptyJSONSourceV1(t *testing.T) {
+	resp, err := PostJSONSourceV1(testState.socket, "")
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
+// POST an invalid JSON source using V0 API
 func TestPOSTInvalidJSONSourceV0(t *testing.T) {
 	// Missing quote in url
 	source := `{
@@ -62,7 +69,26 @@ func TestPOSTInvalidJSONSourceV0(t *testing.T) {
 	require.False(t, resp.Status, "did not return an error")
 }
 
-// POST a new TOML source
+// POST an invalid JSON source using V1 API
+func TestPOSTInvalidJSONSourceV1(t *testing.T) {
+	// Missing quote in url
+	source := `{
+        "id": "package-repo-json-v1",
+		"name": "json package repo",
+		"url": "file://REPO-PATH,
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`
+
+	resp, err := PostJSONSourceV1(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
+// POST a new TOML source using V0 API
 func TestPOSTTOMLSourceV0(t *testing.T) {
 	source := `
 		name = "package-repo-toml-v0"
@@ -84,14 +110,44 @@ func TestPOSTTOMLSourceV0(t *testing.T) {
 	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
 }
 
-// POST an empty TOML source
+// POST a new TOML source using V1 API
+func TestPOSTTOMLSourceV1(t *testing.T) {
+	source := `
+		id = "package-repo-toml-v1"
+		name = "toml package repo"
+		url = "file://REPO-PATH"
+		type = "yum-baseurl"
+		proxy = "https://proxy-url/"
+		check_ssl = true
+		check_gpg = true
+		gpgkey_urls = ["https://url/path/to/gpg-key"]
+	`
+	source = strings.Replace(source, "REPO-PATH", testState.repoDir, 1)
+
+	resp, err := PostTOMLSourceV1(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.True(t, resp.Status, "POST source failed: %#v", resp)
+
+	resp, err = DeleteSourceV1(testState.socket, "package-repo-toml-v1")
+	require.NoError(t, err, "DELETE source failed with a client error")
+	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+}
+
+// POST an empty TOML source using V0 API
 func TestPOSTEmptyTOMLSourceV0(t *testing.T) {
 	resp, err := PostTOMLSourceV0(testState.socket, "")
 	require.NoError(t, err, "POST source failed with a client error")
 	require.False(t, resp.Status, "did not return an error")
 }
 
-// POST an invalid TOML source
+// POST an empty TOML source using V1 API
+func TestPOSTEmptyTOMLSourceV1(t *testing.T) {
+	resp, err := PostTOMLSourceV1(testState.socket, "")
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
+// POST an invalid TOML source using V0 API
 func TestPOSTInvalidTOMLSourceV0(t *testing.T) {
 	// Missing quote in url
 	source := `
@@ -109,7 +165,26 @@ func TestPOSTInvalidTOMLSourceV0(t *testing.T) {
 	require.False(t, resp.Status, "did not return an error")
 }
 
-// POST a wrong TOML source
+// POST an invalid TOML source using V1 API
+func TestPOSTInvalidTOMLSourceV1(t *testing.T) {
+	// Missing quote in url
+	source := `
+		id = "package-repo-toml-v1"
+		name = "toml package repo"
+		url = "file://REPO-PATH
+		type = "yum-baseurl"
+		proxy = "https://proxy-url/"
+		check_ssl = true
+		check_gpg = true
+		gpgkey_urls = ["https://url/path/to/gpg-key"]
+	`
+
+	resp, err := PostTOMLSourceV1(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
+// POST a wrong TOML source using V0 API
 func TestPOSTWrongTOMLSourceV0(t *testing.T) {
 	// Should not have a [] section
 	source := `
@@ -128,7 +203,27 @@ func TestPOSTWrongTOMLSourceV0(t *testing.T) {
 	require.False(t, resp.Status, "did not return an error")
 }
 
-// list sources
+// POST a wrong TOML source using V1 API
+func TestPOSTWrongTOMLSourceV1(t *testing.T) {
+	// Should not have a [] section
+	source := `
+		[package-repo-toml-v1]
+		id = "package-repo-toml-v1"
+		name = "toml package repo"
+		url = "file://REPO-PATH
+		type = "yum-baseurl"
+		proxy = "https://proxy-url/"
+		check_ssl = true
+		check_gpg = true
+		gpgkey_urls = ["https://url/path/to/gpg-key"]
+	`
+
+	resp, err := PostTOMLSourceV1(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
+// list sources using the v0 API
 func TestListSourcesV0(t *testing.T) {
 	sources := []string{`{
 		"name": "package-repo-1",
@@ -174,7 +269,55 @@ func TestListSourcesV0(t *testing.T) {
 	require.Contains(t, list, "package-repo-2")
 }
 
-// Get the source info
+// list sources using the v1 API
+func TestListSourcesV1(t *testing.T) {
+	sources := []string{`{
+		"id": "package-repo-1",
+		"name": "First test package repo",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`,
+		`{
+		"id": "package-repo-2",
+		"name": "Second test package repo",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`}
+
+	for i := range sources {
+		source := strings.Replace(sources[i], "REPO-PATH", testState.repoDir, 1)
+		resp, err := PostJSONSourceV1(testState.socket, source)
+		require.NoError(t, err, "POST source failed with a client error")
+		require.True(t, resp.Status, "POST source failed: %#v", resp)
+	}
+
+	// Remove the test sources, ignoring any errors
+	defer func() {
+		for _, n := range []string{"package-repo-1", "package-repo-2"} {
+			resp, err := DeleteSourceV1(testState.socket, n)
+			require.NoError(t, err, "DELETE source failed with a client error")
+			require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+		}
+	}()
+
+	// Get the list of sources
+	list, api, err := ListSourcesV1(testState.socket)
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, api, "ListSources failed: %#v", api)
+	require.True(t, len(list) > 1, "Not enough sources returned")
+	require.Contains(t, list, "package-repo-1")
+	require.Contains(t, list, "package-repo-2")
+}
+
+// Get the source info using the v0 API
 func TestGetSourceInfoV0(t *testing.T) {
 	source := `
 		name = "package-repo-info-v0"
@@ -203,7 +346,37 @@ func TestGetSourceInfoV0(t *testing.T) {
 	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
 }
 
-func UploadUserDefinedSources(t *testing.T, sources []string) {
+// Get the source info using the v1 API
+func TestGetSourceInfoV1(t *testing.T) {
+	source := `
+		id = "package-repo-info-v1"
+		name = "repo for info test v1"
+		url = "file://REPO-PATH"
+		type = "yum-baseurl"
+		proxy = "https://proxy-url/"
+		check_ssl = true
+		check_gpg = true
+		gpgkey_urls = ["https://url/path/to/gpg-key"]
+	`
+	source = strings.Replace(source, "REPO-PATH", testState.repoDir, 1)
+
+	resp, err := PostTOMLSourceV1(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.True(t, resp.Status, "POST source failed: %#v", resp)
+
+	info, resp, err := GetSourceInfoV1(testState.socket, "package-repo-info-v1")
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, resp, "GET source failed: %#v", resp)
+	require.Contains(t, info, "package-repo-info-v1", "No source info returned")
+	require.Equal(t, "repo for info test v1", info["package-repo-info-v1"].Name)
+	require.Equal(t, "file://"+testState.repoDir, info["package-repo-info-v1"].URL)
+
+	resp, err = DeleteSourceV1(testState.socket, "package-repo-info-v1")
+	require.NoError(t, err, "DELETE source failed with a client error")
+	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+}
+
+func UploadUserDefinedSourcesV0(t *testing.T, sources []string) {
 	for i := range sources {
 		source := strings.Replace(sources[i], "REPO-PATH", testState.repoDir, 1)
 		resp, err := PostJSONSourceV0(testState.socket, source)
@@ -212,8 +385,17 @@ func UploadUserDefinedSources(t *testing.T, sources []string) {
 	}
 }
 
+func UploadUserDefinedSourcesV1(t *testing.T, sources []string) {
+	for i := range sources {
+		source := strings.Replace(sources[i], "REPO-PATH", testState.repoDir, 1)
+		resp, err := PostJSONSourceV1(testState.socket, source)
+		require.NoError(t, err, "POST source failed with a client error")
+		require.True(t, resp.Status, "POST source failed: %#v", resp)
+	}
+}
+
 // verify user defined sources are not present
-func VerifyNoUserDefinedSources(t *testing.T, source_names []string) {
+func VerifyNoUserDefinedSourcesV0(t *testing.T, source_names []string) {
 	list, api, err := ListSourcesV0(testState.socket)
 	require.NoError(t, err, "GET source failed with a client error")
 	require.Nil(t, api, "ListSources failed: %#v", api)
@@ -223,6 +405,17 @@ func VerifyNoUserDefinedSources(t *testing.T, source_names []string) {
 	}
 }
 
+// verify user defined sources are not present
+func VerifyNoUserDefinedSourcesV1(t *testing.T, source_names []string) {
+	list, api, err := ListSourcesV1(testState.socket)
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, api, "ListSources failed: %#v", api)
+	require.GreaterOrEqual(t, len(list), 1, "Not enough sources returned")
+	for i := range source_names {
+		require.NotContains(t, list, source_names[i])
+	}
+}
+
 func TestDeleteUserDefinedSourcesV0(t *testing.T) {
 	source_names := []string{"package-repo-1", "package-repo-2"}
 	sources := []string{`{
@@ -245,10 +438,10 @@ func TestDeleteUserDefinedSourcesV0(t *testing.T) {
 	}`}
 
 	// verify test starts without user defined sources
-	VerifyNoUserDefinedSources(t, source_names)
+	VerifyNoUserDefinedSourcesV0(t, source_names)
 
 	// post user defined sources
-	UploadUserDefinedSources(t, sources)
+	UploadUserDefinedSourcesV0(t, sources)
 	// note: not verifying user defined sources have been pushed b/c correct
 	// operation of PostJSONSourceV0 is validated in the test functions above
 
@@ -260,7 +453,49 @@ func TestDeleteUserDefinedSourcesV0(t *testing.T) {
 	}
 
 	// verify removed sources are not present after removal
-	VerifyNoUserDefinedSources(t, source_names)
+	VerifyNoUserDefinedSourcesV0(t, source_names)
+}
+
+func TestDeleteUserDefinedSourcesV1(t *testing.T) {
+	source_names := []string{"package-repo-1", "package-repo-2"}
+	sources := []string{`{
+		"id": "package-repo-1",
+		"name": "First test package repo",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`,
+		`{
+		"id": "package-repo-2",
+		"name": "Second test package repo",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`}
+
+	// verify test starts without user defined sources
+	VerifyNoUserDefinedSourcesV1(t, source_names)
+
+	// post user defined sources
+	UploadUserDefinedSourcesV1(t, sources)
+	// note: not verifying user defined sources have been pushed b/c correct
+	// operation of PostJSONSourceV0 is validated in the test functions above
+
+	// Remove the test sources
+	for _, n := range source_names {
+		resp, err := DeleteSourceV1(testState.socket, n)
+		require.NoError(t, err, "DELETE source failed with a client error")
+		require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+	}
+
+	// verify removed sources are not present after removal
+	VerifyNoUserDefinedSourcesV0(t, source_names)
 }
 
 func Index(vs []string, t string) int {
@@ -291,7 +526,7 @@ func TestDeleteSystemSourcesV0(t *testing.T) {
 		// try removing system source
 		resp, err := DeleteSourceV0(testState.socket, repo_name)
 		require.NoError(t, err, "DELETE source failed with a client error")
-		require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+		require.False(t, resp.Status, "DELETE system source test failed: %#v", resp)
 
 		// verify that system sources are still there
 		list, api, err := ListSourcesV0(testState.socket)
@@ -301,3 +536,29 @@ func TestDeleteSystemSourcesV0(t *testing.T) {
 		require.Contains(t, list, repo_name)
 	}
 }
+
+func TestDeleteSystemSourcesV1(t *testing.T) {
+	sources_list, api, err := ListSourcesV1(testState.socket)
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, api, "ListSources failed: %#v", api)
+
+	for _, repo_name := range []string{"test-system-repo", "fedora", "baseos"} {
+		// skip repository names which are not present b/c this test can be
+		// executed both as a unit test and as an integration test
+		if !Include(sources_list, repo_name) {
+			continue
+		}
+
+		// try removing system source
+		resp, err := DeleteSourceV1(testState.socket, repo_name)
+		require.NoError(t, err, "DELETE source failed with a client error")
+		require.False(t, resp.Status, "DELETE system source test failed: %#v", resp)
+
+		// verify that system sources are still there
+		list, api, err := ListSourcesV1(testState.socket)
+		require.NoError(t, err, "GET source failed with a client error")
+		require.Nil(t, api, "ListSources failed: %#v", api)
+		require.GreaterOrEqual(t, len(list), 1, "Not enough sources returned")
+		require.Contains(t, list, repo_name)
+	}
+}