From b57814126f83c108fab1da536fffd2e3f1f01e6d Mon Sep 17 00:00:00 2001 From: Emma Turetsky Date: Mon, 1 Apr 2024 19:44:25 +0000 Subject: [PATCH 1/2] getNamespaceByPrefix now checks topology for namespace existence -- Also the namespace struct contains a field determining whether the namespace is from topology --- broker/broker_test.go | 2 + .../20240212192712_create_db_tables.sql | 3 +- registry/registry.go | 6 +++ registry/registry_db.go | 34 +++++++----- registry/registry_db_test.go | 54 +++++++++++++++++++ registry/registry_ui.go | 2 + 6 files changed, 87 insertions(+), 14 deletions(-) diff --git a/broker/broker_test.go b/broker/broker_test.go index db44bb219..0552a1437 100644 --- a/broker/broker_test.go +++ b/broker/broker_test.go @@ -100,6 +100,7 @@ func Setup(t *testing.T, ctx context.Context, egrp *errgroup.Group) { Prefix: "/caches/" + param.Server_Hostname.GetString(), Pubkey: string(keysetBytes), Identity: "test_data", + Topology: false, }) require.NoError(t, err) err = registry.AddNamespace(®istry.Namespace{ @@ -107,6 +108,7 @@ func Setup(t *testing.T, ctx context.Context, egrp *errgroup.Group) { Prefix: "/foo", Pubkey: string(keysetBytes), Identity: "test_data", + Topology: false, }) require.NoError(t, err) diff --git a/registry/migrations/20240212192712_create_db_tables.sql b/registry/migrations/20240212192712_create_db_tables.sql index d80b2e7ce..7d1a73ba9 100644 --- a/registry/migrations/20240212192712_create_db_tables.sql +++ b/registry/migrations/20240212192712_create_db_tables.sql @@ -6,7 +6,8 @@ CREATE TABLE IF NOT EXISTS namespace ( pubkey TEXT NOT NULL, identity TEXT, admin_metadata TEXT CHECK (length("admin_metadata") <= 4000), - custom_fields TEXT CHECK (length("custom_fields") <= 4000) DEFAULT '' + custom_fields TEXT CHECK (length("custom_fields") <= 4000) DEFAULT '', + topology boolean ); CREATE TABLE IF NOT EXISTS topology ( diff --git a/registry/registry.go b/registry/registry.go index 6f552d718..487ff768f 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -307,6 +307,7 @@ func keySignChallengeCommit(ctx *gin.Context, data *registrationData) (bool, map } ns.Pubkey = string(pubkeyData) ns.Identity = data.Identity + ns.Topology = false if data.Identity != "" { idMap := map[string]interface{}{} @@ -855,6 +856,11 @@ func checkNamespaceStatusHandler(ctx *gin.Context) { return } ns, err := getNamespaceByPrefix(req.Prefix) + if ns.Topology { + res := server_structs.CheckNamespaceStatusRes{Approved: true} + ctx.JSON(http.StatusOK, res) + return + } if err != nil || ns == nil { log.Errorf("Error in getNamespaceByPrefix with prefix %s. %v", req.Prefix, err) ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Error getting namespace"}) diff --git a/registry/registry_db.go b/registry/registry_db.go index 9f1a7d425..504f345a8 100644 --- a/registry/registry_db.go +++ b/registry/registry_db.go @@ -80,6 +80,7 @@ type Namespace struct { Identity string `json:"identity" post:"exclude"` AdminMetadata AdminMetadata `json:"admin_metadata" gorm:"serializer:json"` CustomFields map[string]interface{} `json:"custom_fields" gorm:"serializer:json"` + Topology bool `json:"topology" post:"exclude"` } type NamespaceWOPubkey struct { @@ -162,7 +163,7 @@ func IsValidRegStatus(s string) bool { func createTopologyTable() error { err := db.AutoMigrate(&Topology{}) if err != nil { - return fmt.Errorf("Failed to migrate topology table: %v", err) + return fmt.Errorf("failed to migrate topology table: %v", err) } return nil } @@ -171,23 +172,19 @@ func createTopologyTable() error { func namespaceExistsByPrefix(prefix string) (bool, error) { var count int64 - err := db.Model(&Namespace{}).Where("prefix = ?", prefix).Count(&count).Error + err := db.Model(&Topology{}).Where("prefix = ?", prefix).Count(&count).Error if err != nil { return false, err } if count > 0 { return true, nil } else { - if config.GetPreferredPrefix() == "OSDF" { - // Perform a count across both 'namespace' and 'topology' tables - err := db.Model(&Topology{}).Where("prefix = ?", prefix).Count(&count).Error - if err != nil { - return false, err - } - return count > 0, nil + err = db.Model(&Namespace{}).Where("prefix = ?", prefix).Count(&count).Error + if err != nil { + return false, err } + return count > 0, nil } - return false, nil } func namespaceSupSubChecks(prefix string) (superspaces []string, subspaces []string, inTopo bool, err error) { @@ -336,12 +333,23 @@ func getNamespaceByPrefix(prefix string) (*Namespace, error) { if prefix == "" { return nil, errors.New("Invalid prefix. Prefix must not be empty") } - ns := Namespace{} - err := db.Where("prefix = ? ", prefix).Last(&ns).Error + tp := Topology{} + err := db.Where("prefix = ?", prefix).Last(&tp).Error + var ns = Namespace{} if errors.Is(err, gorm.ErrRecordNotFound) { - return nil, fmt.Errorf("namespace with id %q not found in database", prefix) + err := db.Where("prefix = ? ", prefix).Last(&ns).Error + ns.Topology = false + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, fmt.Errorf("namespace with id %q not found in database", prefix) + } else if err != nil { + return nil, errors.Wrap(err, "error retrieving pubkey") + } } else if err != nil { return nil, errors.Wrap(err, "error retrieving pubkey") + } else { + ns.ID = tp.ID + ns.Prefix = tp.Prefix + ns.Topology = true } // By default, JSON unmarshal will convert any generic number to float diff --git a/registry/registry_db_test.go b/registry/registry_db_test.go index 521de598b..341b1c260 100644 --- a/registry/registry_db_test.go +++ b/registry/registry_db_test.go @@ -118,6 +118,7 @@ func mockNamespace(prefix, pubkey, identity string, adminMetadata AdminMetadata) Pubkey: pubkey, Identity: identity, AdminMetadata: adminMetadata, + Topology: false, } } @@ -732,6 +733,59 @@ func topologyMockup(t *testing.T, namespaces []string) *httptest.Server { return svr } +func TestGetNamespaceByPrefix(t *testing.T) { + ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) + defer func() { require.NoError(t, egrp.Wait()) }() + defer cancel() + + viper.Reset() + + topoNamespaces := []string{"/topo/foo"} + svr := topologyMockup(t, topoNamespaces) + defer svr.Close() + + registryDB := t.TempDir() + viper.Set("Registry.DbLocation", filepath.Join(registryDB, "test.sqlite")) + viper.Set("Federation.TopologyNamespaceURL", svr.URL) + config.InitConfig() + + err := InitializeDB(ctx) + require.NoError(t, err) + defer func() { + err := ShutdownDB() + assert.NoError(t, err) + }() + + //Test topology table population + err = createTopologyTable() + require.NoError(t, err) + err = PopulateTopology() + require.NoError(t, err) + + //Test that getNamespaceByPrefix wraps a pelican namespace around a topology one + ns, err := getNamespaceByPrefix("/topo/foo") + require.NoError(t, err) + require.True(t, ns.Topology) + + // Add a test namespace so we can test that checkExists still works + new_ns := Namespace{ + ID: 0, + Prefix: "/regular/foo", + Pubkey: "", + Identity: "", + AdminMetadata: AdminMetadata{}, + } + err = AddNamespace(&new_ns) + require.NoError(t, err) + + //Test that getNamespaceByPrefix returns namespace with a false topology variable + ns_reg, err := getNamespaceByPrefix("/regular/foo") + require.NoError(t, err) + require.False(t, ns_reg.Topology) + + viper.Reset() +} + func TestRegistryTopology(t *testing.T) { ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) defer func() { require.NoError(t, egrp.Wait()) }() diff --git a/registry/registry_ui.go b/registry/registry_ui.go index a303d79b4..35fbb9a13 100644 --- a/registry/registry_ui.go +++ b/registry/registry_ui.go @@ -422,6 +422,7 @@ func createUpdateNamespace(ctx *gin.Context, isUpdate bool) { } ns := Namespace{} + ns.Topology = false if ctx.ShouldBindJSON(&ns) != nil { ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid create or update namespace request"}) return @@ -441,6 +442,7 @@ func createUpdateNamespace(ctx *gin.Context, isUpdate bool) { return } ns.Prefix = updated_prefix + ns.Topology = false if !isUpdate { // Check if prefix exists before doing anything else. Skip check if it's update operation From 1422426a65738d29c831fd559a10ba03d9e2d919 Mon Sep 17 00:00:00 2001 From: Emma Turetsky Date: Tue, 2 Apr 2024 19:08:19 +0000 Subject: [PATCH 2/2] Address PR comments --- broker/broker_test.go | 2 -- .../20240212192712_create_db_tables.sql | 3 +-- registry/registry_db.go | 17 +++++++++++++---- registry/registry_db_test.go | 1 - 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/broker/broker_test.go b/broker/broker_test.go index 0552a1437..db44bb219 100644 --- a/broker/broker_test.go +++ b/broker/broker_test.go @@ -100,7 +100,6 @@ func Setup(t *testing.T, ctx context.Context, egrp *errgroup.Group) { Prefix: "/caches/" + param.Server_Hostname.GetString(), Pubkey: string(keysetBytes), Identity: "test_data", - Topology: false, }) require.NoError(t, err) err = registry.AddNamespace(®istry.Namespace{ @@ -108,7 +107,6 @@ func Setup(t *testing.T, ctx context.Context, egrp *errgroup.Group) { Prefix: "/foo", Pubkey: string(keysetBytes), Identity: "test_data", - Topology: false, }) require.NoError(t, err) diff --git a/registry/migrations/20240212192712_create_db_tables.sql b/registry/migrations/20240212192712_create_db_tables.sql index 7d1a73ba9..d80b2e7ce 100644 --- a/registry/migrations/20240212192712_create_db_tables.sql +++ b/registry/migrations/20240212192712_create_db_tables.sql @@ -6,8 +6,7 @@ CREATE TABLE IF NOT EXISTS namespace ( pubkey TEXT NOT NULL, identity TEXT, admin_metadata TEXT CHECK (length("admin_metadata") <= 4000), - custom_fields TEXT CHECK (length("custom_fields") <= 4000) DEFAULT '', - topology boolean + custom_fields TEXT CHECK (length("custom_fields") <= 4000) DEFAULT '' ); CREATE TABLE IF NOT EXISTS topology ( diff --git a/registry/registry_db.go b/registry/registry_db.go index 504f345a8..031ff14de 100644 --- a/registry/registry_db.go +++ b/registry/registry_db.go @@ -80,7 +80,7 @@ type Namespace struct { Identity string `json:"identity" post:"exclude"` AdminMetadata AdminMetadata `json:"admin_metadata" gorm:"serializer:json"` CustomFields map[string]interface{} `json:"custom_fields" gorm:"serializer:json"` - Topology bool `json:"topology" post:"exclude"` + Topology bool `json:"topology" gorm:"-:all" post:"exclude"` //This field is an extra field that's not included in the db } type NamespaceWOPubkey struct { @@ -270,6 +270,11 @@ func getNamespaceJwksById(id int) (jwk.Set, error) { } func getNamespaceJwksByPrefix(prefix string) (jwk.Set, *AdminMetadata, error) { + // Note that this cannot retrieve public keys from topology as the topology table + // doesn't contain that information. + if prefix == "" { + return nil, nil, errors.New("Invalid prefix. Prefix must not be empty") + } var result Namespace err := db.Select("pubkey", "admin_metadata").Where("prefix = ?", prefix).Last(&result).Error if errors.Is(err, gorm.ErrRecordNotFound) { @@ -330,6 +335,10 @@ func getNamespaceById(id int) (*Namespace, error) { } func getNamespaceByPrefix(prefix string) (*Namespace, error) { + // This function will check the topology table first to see if the + // namespace exists there before checking the namespace table + // If the namespace exists in the topology table, it will be wrapped + // as a pelican namespace before being returned if prefix == "" { return nil, errors.New("Invalid prefix. Prefix must not be empty") } @@ -340,12 +349,12 @@ func getNamespaceByPrefix(prefix string) (*Namespace, error) { err := db.Where("prefix = ? ", prefix).Last(&ns).Error ns.Topology = false if errors.Is(err, gorm.ErrRecordNotFound) { - return nil, fmt.Errorf("namespace with id %q not found in database", prefix) + return nil, fmt.Errorf("namespace with prefix %q not found in database", prefix) } else if err != nil { - return nil, errors.Wrap(err, "error retrieving pubkey") + return nil, errors.Wrap(err, "error retrieving the namespace by its prefix") } } else if err != nil { - return nil, errors.Wrap(err, "error retrieving pubkey") + return nil, errors.Wrap(err, "error retrieving the namespace by its prefix") } else { ns.ID = tp.ID ns.Prefix = tp.Prefix diff --git a/registry/registry_db_test.go b/registry/registry_db_test.go index 341b1c260..696944953 100644 --- a/registry/registry_db_test.go +++ b/registry/registry_db_test.go @@ -118,7 +118,6 @@ func mockNamespace(prefix, pubkey, identity string, adminMetadata AdminMetadata) Pubkey: pubkey, Identity: identity, AdminMetadata: adminMetadata, - Topology: false, } }