Fix catalog access (#537)

* Remove URL checks from CreateCatalogFromSubscriptionAsync
* Add client methods to retrieve AdminCatalogs
    These methods do not require a full Organization object
    * GetAdminCatalogByHref
    * GetAdminCatalogById
    * GetAdminCatalogByName

* Add client methods to retrieve catalogs
    These methods do not require a full Organization object
    * GetCatalogByHref
    * GetCatalogById
    * GetCatalogByName

* Add method to retrieve vApp template record
    Given a vApp template object, this method retrieves the corresponding
    query record

* Upgrade YAML dependency

Signed-off-by: Giuseppe Maxia <[email protected]>

.changes/v2.19.0/537-bug-fixes.md

@@ -0,0 +1 @@
+* Remove URL checks from `CreateCatalogFromSubscriptionAsync` to allow catalog creation from non-VCD entities, such as vSphere shared library [GH-537]

.changes/v2.19.0/537-features.md

@@ -0,0 +1,3 @@
+* Added client methods `GetCatalogByHref`, `GetCatalogById`, `GetCatalogByName` to retrieve Catalogs without an Org object [GH-537]
+* Added client methods `GetAdminCatalogByHref`, `GetAdminCatalogById`, `GetAdminCatalogByName` to retrieve AdminCatalogs without an AdminOrg object [GH-537]
+* Added method `VAppTemplate.GetVappTemplateRecord` to retrieve a VAppTemplate query record [GH-537]

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.19.0 (TBC)
+
+Changes in progress for v2.19.0 are available at [.changes/v2.19.0](https://github.com/vmware/go-vcloud-director/tree/main/.changes/v2.19.0) until the release.
+
 ## 2.18.0 (December 14, 2022)
 
 ### FEATURES

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/kr/pretty v0.2.1
 	github.com/peterhellberg/link v1.1.0
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
-	gopkg.in/yaml.v2 v2.2.2
+	gopkg.in/yaml.v2 v2.4.0
 )
 
 require (

govcd/access_control_catalog_test.go

@@ -354,7 +354,7 @@ func (vcd *TestVCD) testCatalogAccessControl(adminOrg *AdminOrg, catalog accessC
 		catalogs, err := vcd.client.Client.QueryCatalogRecords(catalogName, TenantContext{newOrg.AdminOrg.ID, newOrg.AdminOrg.Name})
 		check.Assert(err, IsNil)
 		check.Assert(len(catalogs), Equals, 1)
-		foundCatalog, err := vcd.client.Client.GetCatalogByHref(catalogs[0].HREF)
+		foundCatalog, err := vcd.client.Client.GetAdminCatalogByHref(catalogs[0].HREF)
 		check.Assert(err, IsNil)
 		check.Assert(foundCatalog.AdminCatalog.ID, Equals, catalog.GetId())
 	}

govcd/admincatalog.go

@@ -175,26 +175,9 @@ func (org *AdminOrg) CreateCatalogFromSubscriptionAsync(subscription types.Exter
 		},
 	}
 
-	uuid := extractUuid(subscription.Location)
-	if uuid == "" {
-		return nil, fmt.Errorf("subscription URL %s does not contain a valid UUID", subscription.Location)
-	}
-	subscription.Location = strings.TrimSpace(subscription.Location)
-	if !strings.HasSuffix(subscription.Location, "/") {
-		return nil, fmt.Errorf("subscription URL '%s' should end with a '/'", subscription.Location)
-	}
-
-	// The subscription URL returned by the API is in abbreviated form
-	// such as "/vcsp/lib/65637586-c703-48ae-a7e2-82605d18db57/"
-	// If the passed URL is so abbreviated, we need to add the host
-	subscriptionUrl, err := buildFullUrl(subscription.Location, org.AdminOrg.HREF)
-	if err != nil {
-		return nil, fmt.Errorf("error composing subscription URL: %s", err)
-	}
-	adminCatalog.AdminCatalog.ExternalCatalogSubscription.Location = subscriptionUrl
 	adminCatalog.AdminCatalog.ExternalCatalogSubscription.Password = password
 	adminCatalog.AdminCatalog.ExternalCatalogSubscription.LocalCopy = localCopy
-	_, err = org.client.ExecuteRequest(href, http.MethodPost, types.MimeAdminCatalog,
+	_, err := org.client.ExecuteRequest(href, http.MethodPost, types.MimeAdminCatalog,
 		"error subscribing to catalog: %s", adminCatalog.AdminCatalog, adminCatalog.AdminCatalog)
 	if err != nil {
 		return nil, err
@@ -628,8 +611,8 @@ func (catalog *AdminCatalog) QueryTaskList(filter map[string]string) ([]*types.Q
 	return catalog.client.QueryTaskList(filter)
 }
 
-// GetCatalogByHref allows retrieving a catalog from HREF, without its parent
-func (client *Client) GetCatalogByHref(catalogHref string) (*AdminCatalog, error) {
+// GetAdminCatalogByHref allows retrieving a catalog from HREF, without a fully qualified AdminOrg object
+func (client *Client) GetAdminCatalogByHref(catalogHref string) (*AdminCatalog, error) {
 	catalogHref = strings.Replace(catalogHref, "/api/catalog", "/api/admin/catalog", 1)
 
 	cat := NewAdminCatalog(client)
@@ -680,3 +663,34 @@ func (client *Client) QueryCatalogRecords(name string, ctx TenantContext) ([]*ty
 	util.Logger.Printf("[DEBUG] QueryCatalogRecords returned with : %#v (%d) and error: %v", catalogs, len(catalogs), err)
 	return catalogs, nil
 }
+
+// GetAdminCatalogById allows retrieving a catalog from ID, without a fully qualified AdminOrg object
+func (client *Client) GetAdminCatalogById(catalogId string) (*AdminCatalog, error) {
+	href, err := url.JoinPath(client.VCDHREF.String(), "admin", "catalog", extractUuid(catalogId))
+	if err != nil {
+		return nil, err
+	}
+	return client.GetAdminCatalogByHref(href)
+}
+
+// GetAdminCatalogByName allows retrieving a catalog from name, without a fully qualified AdminOrg object
+func (client *Client) GetAdminCatalogByName(parentOrg, catalogName string) (*AdminCatalog, error) {
+	catalogs, err := queryCatalogList(client, nil)
+	if err != nil {
+		return nil, err
+	}
+	var parentOrgs []string
+	for _, cat := range catalogs {
+		if cat.Name == catalogName && cat.OrgName == parentOrg {
+			return client.GetAdminCatalogByHref(cat.HREF)
+		}
+		if cat.Name == catalogName {
+			parentOrgs = append(parentOrgs, cat.OrgName)
+		}
+	}
+	parents := ""
+	if len(parentOrgs) > 0 {
+		parents = fmt.Sprintf(" - Found catalog %s in Orgs %v", catalogName, parentOrgs)
+	}
+	return nil, fmt.Errorf("no catalog '%s' found in Org %s%s", catalogName, parentOrg, parents)
+}

govcd/catalog.go

@@ -1139,3 +1139,50 @@ func (catalog *Catalog) QueryTaskList(filter map[string]string) ([]*types.QueryR
 	}
 	return catalog.client.QueryTaskList(newFilter)
 }
+
+// GetCatalogByHref allows retrieving a catalog from HREF, without a fully qualified Org object
+func (client *Client) GetCatalogByHref(catalogHref string) (*Catalog, error) {
+	catalogHref = strings.Replace(catalogHref, "/api/admin/catalog", "/api/catalog", 1)
+
+	cat := NewCatalog(client)
+
+	_, err := client.ExecuteRequest(catalogHref, http.MethodGet,
+		"", "error retrieving catalog: %s", nil, cat.Catalog)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return cat, nil
+}
+
+// GetCatalogById allows retrieving a catalog from ID, without a fully qualified Org object
+func (client *Client) GetCatalogById(catalogId string) (*Catalog, error) {
+	href, err := url.JoinPath(client.VCDHREF.String(), "catalog", extractUuid(catalogId))
+	if err != nil {
+		return nil, err
+	}
+	return client.GetCatalogByHref(href)
+}
+
+// GetCatalogByName allows retrieving a catalog from name, without a fully qualified Org object
+func (client *Client) GetCatalogByName(parentOrg, catalogName string) (*Catalog, error) {
+	catalogs, err := queryCatalogList(client, nil)
+	if err != nil {
+		return nil, err
+	}
+	var parentOrgs []string
+	for _, cat := range catalogs {
+		if cat.Name == catalogName && cat.OrgName == parentOrg {
+			return client.GetCatalogByHref(cat.HREF)
+		}
+		if cat.Name == catalogName {
+			parentOrgs = append(parentOrgs, cat.OrgName)
+		}
+	}
+	parents := ""
+	if len(parentOrgs) > 0 {
+		parents = fmt.Sprintf(" - Found catalog %s in Orgs %v", catalogName, parentOrgs)
+	}
+	return nil, fmt.Errorf("no catalog '%s' found in Org %s%s", catalogName, parentOrg, parents)
+}

govcd/catalog_subscription_test.go

@@ -9,6 +9,7 @@ package govcd
 
 import (
 	"fmt"
+	"net/url"
 	"strings"
 	"time"
 
@@ -200,14 +201,18 @@ func testSubscribedCatalog(testData subscriptionTestData, check *C) {
 	err = fromCatalog.Refresh()
 	check.Assert(err, IsNil)
 
+	subscriptionUrl, err := fromCatalog.FullSubscriptionUrl()
+	check.Assert(err, IsNil)
+
 	subscriptionParams := types.ExternalCatalogSubscription{
 		SubscribeToExternalFeeds: true,
-		Location:                 fromCatalog.AdminCatalog.PublishExternalCatalogParams.CatalogPublishedUrl,
+		Location:                 subscriptionUrl,
 		Password:                 subscriptionPassword,
 		LocalCopy:                testData.localCopy,
 	}
 
 	var toCatalog *AdminCatalog
+	testSubscribedCatalogWithInvalidParameters(toOrg, subscriptionParams, subscribingCatalogName, subscriptionPassword, testData.localCopy, check)
 	if testData.asynchronousSubscription {
 		drawHeader("-", "creating subscribed catalog asynchronously")
 		// With asynchronous subscription the catalog starts the subscription but does not report its state, which is
@@ -346,3 +351,29 @@ func testMonitor(task *types.Task) {
 		fmt.Print(marker)
 	}
 }
+
+func testSubscribedCatalogWithInvalidParameters(org *AdminOrg, subscription types.ExternalCatalogSubscription,
+	name, password string, localCopy bool, check *C) {
+
+	uuid := extractUuid(subscription.Location)
+	params := subscription
+	params.Location = strings.Replace(params.Location, uuid, "deadbeef-d72f-4a21-a4d2-4dc9e0b36555", 1)
+	// Use a valid host with invalid UUID
+	_, err := org.CreateCatalogFromSubscriptionAsync(params, nil, name, password, localCopy)
+	check.Assert(err, ErrorMatches, ".*RESOURCE_NOT_FOUND.*")
+
+	newUrl, err := url.Parse(subscription.Location)
+	check.Assert(err, IsNil)
+
+	params = subscription
+	params.Location = strings.Replace(params.Location, newUrl.Host, "fake.example.com", 1)
+	// use an invalid host
+	_, err = org.CreateCatalogFromSubscriptionAsync(params, nil, name, password, localCopy)
+	check.Assert(err, ErrorMatches, ".*INVALID_URL_OR_PASSWORD.*")
+
+	params = subscription
+	params.Location = "not-an-URL"
+	// use an invalid URL
+	_, err = org.CreateCatalogFromSubscriptionAsync(params, nil, name, password, localCopy)
+	check.Assert(err, ErrorMatches, ".*UNKNOWN_ERROR.*")
+}

govcd/catalog_test.go

@@ -1058,3 +1058,174 @@ func (vcd *TestVCD) Test_CatalogUploadMediaImageWihUdfTypeIso(check *C) {
 	// Delete testing catalog item
 	deleteCatalogItem(check, catalog, mediaName)
 }
+
+func (vcd *TestVCD) Test_GetAdminCatalogById(check *C) {
+	if vcd.config.VCD.Org == "" || vcd.config.VCD.Catalog.Name == "" {
+		check.Skip("no Org or Catalog found in configuration")
+	}
+
+	// 1. Get a catalog from an organization
+	org, err := vcd.client.GetAdminOrgByName(vcd.config.VCD.Org)
+	check.Assert(err, IsNil)
+
+	adminCatalog, err := org.GetAdminCatalogByName(vcd.config.VCD.Catalog.Name, false)
+	check.Assert(err, IsNil)
+
+	// 2. retrieve that same catalog from the client alone using HREF
+	adminCatalogByHref, err := vcd.client.Client.GetAdminCatalogByHref(adminCatalog.AdminCatalog.HREF)
+	check.Assert(err, IsNil)
+	check.Assert(adminCatalogByHref.AdminCatalog.HREF, Equals, adminCatalog.AdminCatalog.HREF)
+
+	// 3. retrieve the same catalog again, using ID
+	adminCatalogById, err := vcd.client.Client.GetAdminCatalogById(adminCatalog.AdminCatalog.ID)
+	check.Assert(err, IsNil)
+	check.Assert(adminCatalogById.AdminCatalog.HREF, Equals, adminCatalog.AdminCatalog.HREF)
+}
+
+func (vcd *TestVCD) Test_CatalogAccessAsOrgUsers(check *C) {
+	if vcd.config.Tenants == nil || len(vcd.config.Tenants) < 2 {
+		check.Skip("no tenants found in configuration")
+	}
+
+	if vcd.config.OVA.OvaPath == "" || vcd.config.Media.MediaPath == "" {
+		check.Skip("no OVA or Media path found in configuration")
+	}
+
+	org1Name := vcd.config.Tenants[0].SysOrg
+	user1Name := vcd.config.Tenants[0].User
+	password1 := vcd.config.Tenants[0].Password
+	org2Name := vcd.config.Tenants[1].SysOrg
+	user2Name := vcd.config.Tenants[1].User
+	password2 := vcd.config.Tenants[1].Password
+
+	org1AsSystem, err := vcd.client.GetAdminOrgByName(org1Name)
+	check.Assert(err, IsNil)
+	check.Assert(org1AsSystem, NotNil)
+
+	org2AsSystem, err := vcd.client.GetAdminOrgByName(org2Name)
+	if err != nil {
+		if ContainsNotFound(err) {
+			check.Skip(fmt.Sprintf("organization %s not found", org2Name))
+		}
+	}
+	check.Assert(err, IsNil)
+	check.Assert(org2AsSystem, NotNil)
+	vcdClient1 := NewVCDClient(vcd.client.Client.VCDHREF, true)
+	err = vcdClient1.Authenticate(user1Name, password1, org1Name)
+	check.Assert(err, IsNil)
+
+	vcdClient2 := NewVCDClient(vcd.client.Client.VCDHREF, true)
+	err = vcdClient2.Authenticate(user2Name, password2, org2Name)
+	check.Assert(err, IsNil)
+
+	org1, err := vcdClient1.GetOrgByName(org1Name)
+	check.Assert(err, IsNil)
+	org2, err := vcdClient2.GetOrgByName(org2Name)
+	check.Assert(err, IsNil)
+	check.Assert(org2, NotNil)
+	catalogName := check.TestName() + "-cat"
+	fmt.Printf("creating catalog %s in org %s\n", catalogName, org1Name)
+	adminCatalog1AsSystem, err := org1AsSystem.CreateCatalog(catalogName, fmt.Sprintf("catalog %s created in %s", catalogName, org1Name))
+	check.Assert(err, IsNil)
+	AddToCleanupList(catalogName, "catalog", org1Name, check.TestName())
+	catalog1AsSystem, err := org1AsSystem.GetCatalogByName(catalogName, true)
+	check.Assert(err, IsNil)
+	fmt.Printf("sharing catalog %s from org %s\n", catalogName, org1Name)
+	err = adminCatalog1AsSystem.SetAccessControl(&types.ControlAccessParams{
+		IsSharedToEveryone: false,
+		AccessSettings: &types.AccessSettingList{
+			[]*types.AccessSetting{
+				{
+					Subject: &types.LocalSubject{
+						HREF: org2.Org.HREF,
+						Name: org2Name,
+						Type: types.MimeOrg,
+					},
+					AccessLevel: types.ControlAccessReadOnly,
+				},
+			},
+		},
+	}, true)
+	check.Assert(err, IsNil)
+
+	// populate the catalog
+
+	vappTemplateName := check.TestName() + "-template"
+	mediaName := check.TestName() + "-media"
+	fmt.Printf("uploading vApp template into catalog %s\n", catalogName)
+	task, err := catalog1AsSystem.UploadOvf(vcd.config.OVA.OvaPath, vappTemplateName, vappTemplateName, 1024)
+	check.Assert(err, IsNil)
+	err = task.WaitTaskCompletion()
+	check.Assert(err, IsNil)
+
+	fmt.Printf("uploading media image into catalog %s\n", catalogName)
+	uploadTask, err := catalog1AsSystem.UploadMediaImage(mediaName, "upload from test", vcd.config.Media.MediaPath, 1024)
+	check.Assert(err, IsNil)
+	err = uploadTask.WaitTaskCompletion()
+	check.Assert(err, IsNil)
+
+	vAppTemplateAsSystem, err := catalog1AsSystem.GetVAppTemplateByName(vappTemplateName)
+	check.Assert(err, IsNil)
+	check.Assert(vAppTemplateAsSystem, NotNil)
+	mediaRecordAsSystem, err := catalog1AsSystem.GetMediaByName(mediaName, true)
+	check.Assert(err, IsNil)
+	check.Assert(mediaRecordAsSystem, NotNil)
+
+	// Retrieve catalog by ID in its own Org
+	adminCatalog1, err := vcdClient1.Client.GetAdminCatalogById(adminCatalog1AsSystem.AdminCatalog.ID)
+	check.Assert(err, IsNil)
+	check.Assert(adminCatalog1.AdminCatalog.HREF, Equals, adminCatalog1AsSystem.AdminCatalog.HREF)
+
+	catalog1, err := vcdClient1.Client.GetCatalogById(adminCatalog1AsSystem.AdminCatalog.ID)
+	check.Assert(err, IsNil)
+	check.Assert(catalog1.Catalog.HREF, Equals, catalog1AsSystem.Catalog.HREF)
+
+	startTime := time.Now()
+	timeout := 100 * time.Second
+	// Start retrieving catalog in the other org
+	fmt.Printf("retrieving catalog %s in org %s\n", catalogName, org2Name)
+	for time.Since(startTime) < timeout {
+		_, err = vcdClient2.Client.GetAdminCatalogById(adminCatalog1AsSystem.AdminCatalog.ID)
+		if err == nil {
+			fmt.Printf("shared catalog available in %s\n", time.Since(startTime))
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	// Retrieve the shared catalog in the other organization
+	adminCatalog2, err := vcdClient2.Client.GetAdminCatalogById(adminCatalog1AsSystem.AdminCatalog.ID)
+	check.Assert(err, IsNil)
+	check.Assert(adminCatalog2, NotNil)
+
+	// Retrieve the catalog from both tenants, using functions that don't rely on organization internals
+	catalog1FromOrg, err := vcdClient1.Client.GetCatalogByName(org1.Org.Name, catalogName)
+	check.Assert(err, IsNil)
+	adminCatalog1FromOrg, err := vcdClient1.Client.GetAdminCatalogByName(org1.Org.Name, catalogName)
+	check.Assert(err, IsNil)
+	catalog2FromOrg, err := vcdClient2.Client.GetCatalogByName(org1.Org.Name, catalogName)
+	check.Assert(err, IsNil)
+	adminCatalog2FromOrg, err := vcdClient2.Client.GetAdminCatalogByName(org1.Org.Name, catalogName)
+	check.Assert(err, IsNil)
+
+	// Also retrieve the catalog items from both tenants
+	vAppTemplate1, err := catalog1FromOrg.GetVAppTemplateByName(vappTemplateName)
+	check.Assert(err, IsNil)
+	check.Assert(vAppTemplate1.VAppTemplate.HREF, Equals, vAppTemplateAsSystem.VAppTemplate.HREF)
+	mediaRecord1, err := catalog1FromOrg.GetMediaByName(mediaName, false)
+	check.Assert(err, IsNil)
+	check.Assert(mediaRecord1.Media.HREF, Equals, mediaRecordAsSystem.Media.HREF)
+
+	vAppTemplate2, err := catalog2FromOrg.GetVAppTemplateByName(vappTemplateName)
+	check.Assert(err, IsNil)
+	check.Assert(vAppTemplate2.VAppTemplate.HREF, Equals, vAppTemplateAsSystem.VAppTemplate.HREF)
+	mediaRecord2, err := catalog2FromOrg.GetMediaByName(mediaName, false)
+	check.Assert(err, IsNil)
+	check.Assert(mediaRecord2.Media.HREF, Equals, mediaRecordAsSystem.Media.HREF)
+
+	check.Assert(catalog1FromOrg.Catalog.HREF, Equals, catalog1AsSystem.Catalog.HREF)
+	check.Assert(adminCatalog1FromOrg.AdminCatalog.HREF, Equals, adminCatalog1AsSystem.AdminCatalog.HREF)
+	check.Assert(adminCatalog2FromOrg.AdminCatalog.HREF, Equals, adminCatalog1AsSystem.AdminCatalog.HREF)
+	check.Assert(catalog2FromOrg.Catalog.HREF, Equals, catalog1AsSystem.Catalog.HREF)
+	err = adminCatalog1AsSystem.Delete(true, true)
+	check.Assert(err, IsNil)
+}

govcd/org.go

@@ -154,6 +154,7 @@ func (org *Org) CreateCatalogWithStorageProfile(name, description string, storag
 		return nil, err
 	}
 	catalog.Catalog = &adminCatalog.AdminCatalog.Catalog
+	catalog.parent = org
 	return catalog, nil
 }