Skip to content

Commit

Permalink
[#459] Drop the name column
Browse files Browse the repository at this point in the history
  • Loading branch information
fivitti committed Dec 9, 2024
1 parent 00c425a commit 34e3228
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 132 deletions.
2 changes: 0 additions & 2 deletions api/dhcp-defs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,6 @@
type: array
items:
$ref: '#/definitions/LocalSubnet'
name:
type: string

Subnets:
type: object
Expand Down
6 changes: 0 additions & 6 deletions backend/server/database/migrations/61_subnet_user_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,12 @@ func init() {
_, err := db.Exec(`
ALTER TABLE local_subnet
ADD COLUMN user_context JSONB;
ALTER TABLE subnet
ADD COLUMN name TEXT;
`)
return err
}, func(db migrations.DB) error {
_, err := db.Exec(`
ALTER TABLE local_subnet
DROP COLUMN user_context;
ALTER TABLE subnet
DROP COLUMN name;
`)
return err
})
Expand Down
1 change: 0 additions & 1 deletion backend/server/database/model/keaconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ func TestNewSubnetFromKea(t *testing.T) {

require.Equal(t, "bar", parsedSubnet.LocalSubnets[0].UserContext["foo"])
require.Equal(t, "baz", parsedSubnet.LocalSubnets[0].UserContext["subnet-name"])
require.Empty(t, parsedSubnet.Name)
}

// Test that the error is returned when the subnet prefix is invalid.
Expand Down
32 changes: 1 addition & 31 deletions backend/server/database/model/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,6 @@ type Subnet struct {
PdUtilization int16
Stats SubnetStats
StatsCollectedAt time.Time

Name string
}

// Returns local subnet id for the specified daemon.
Expand Down Expand Up @@ -717,7 +715,7 @@ func GetSubnetsByPage(dbi dbops.DBI, offset, limit int64, filters *SubnetsByPage
q = q.WhereOr("text(subnet.prefix) LIKE ?", "%"+*filters.Text+"%").
WhereOr("concat(host(ap.lower_bound), '-', host(ap.upper_bound)) LIKE ?", "%"+*filters.Text+"%").
WhereOr("shared_network.name LIKE ?", "%"+*filters.Text+"%").
WhereOr("subnet.name LIKE ?", "%"+*filters.Text+"%")
WhereOr("ls.user_context->>'subnet-name' LIKE ?", "%"+*filters.Text+"%")
return q, nil
})
}
Expand Down Expand Up @@ -873,9 +871,6 @@ func commitSubnetsIntoDB(tx *pg.Tx, networkID int64, subnets []Subnet) (addedSub
subnet.SharedNetworkID = networkID
}

// Extract the subnet name from the user-context.
subnet.Name = extractSubnetNameFromUserContext(subnet)

if subnet.ID == 0 {
err = AddSubnet(tx, subnet)
if err != nil {
Expand Down Expand Up @@ -905,31 +900,6 @@ func commitSubnetsIntoDB(tx *pg.Tx, networkID int64, subnets []Subnet) (addedSub
return addedSubnets, nil
}

// Extract the subnet name from the user-context.
// Returns the subnet name if found; otherwise, an empty string.
//
// ToDo: We arbitrarily choose the property key that is used to store the
// subnet name in the user-context. It may not fit all deployments. We should
// consider adding a configuration option to specify the key.
//
// Note that the Kea documentation does not specify any convention for
// storing the subnet name in the user-context.
//
// The first name that is found in any related user-context is used as the
// subnet name.
func extractSubnetNameFromUserContext(subnet *Subnet) string {
subnetNameKey := "subnet-name"
for _, localSubnet := range subnet.LocalSubnets {
if name, ok := localSubnet.UserContext[subnetNameKey]; ok {
nameStr, ok := name.(string)
if ok {
return nameStr
}
}
}
return ""
}

// Iterates over the shared networks, subnets and hosts and commits them to the database.
// In addition it associates them with the specified app. Returns a list of added subnets.
// This function runs all database operations in a transaction.
Expand Down
104 changes: 27 additions & 77 deletions backend/server/database/model/subnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,6 @@ func TestGetSubnetsByPage(t *testing.T) {
{
Prefix: "192.0.2.0/24",
SharedNetworkID: sharedNetwork.ID,
Name: "subnet-name",
LocalSubnets: []*LocalSubnet{
{
DaemonID: apps[0].Daemons[0].ID,
Expand All @@ -613,6 +612,9 @@ func TestGetSubnetsByPage(t *testing.T) {
UpperBound: "192.0.2.30",
},
},
UserContext: map[string]interface{}{
"subnet-name": "subnet-name",
},
},
},
},
Expand All @@ -631,6 +633,15 @@ func TestGetSubnetsByPage(t *testing.T) {
UpperBound: "192.0.3.20",
},
},
UserContext: map[string]interface{}{
"subnet-name": "foo",
},
},
{
DaemonID: apps[1].Daemons[0].ID,
UserContext: map[string]interface{}{
"subnet-name": "bar",
},
},
},
},
Expand Down Expand Up @@ -709,6 +720,21 @@ func TestGetSubnetsByPage(t *testing.T) {
require.NoError(t, err)
require.Zero(t, count)
require.Empty(t, returned)

// This should match one subnet.
filters.Text = newPtr("foo")
returned, count, err = GetSubnetsByPage(db, 0, 10, filters, "", SortDirAny)
require.NoError(t, err)
require.EqualValues(t, 1, count)
require.Len(t, returned, 1)
require.Equal(t, "192.0.3.0/24", returned[0].Prefix)

filters.Text = newPtr("bar")
returned, count, err = GetSubnetsByPage(db, 0, 10, filters, "", SortDirAny)
require.NoError(t, err)
require.EqualValues(t, 1, count)
require.Len(t, returned, 1)
require.Equal(t, "192.0.3.0/24", returned[0].Prefix)
}

// Test that the subnet can be fetched by local ID and app ID.
Expand Down Expand Up @@ -967,12 +993,10 @@ func TestCommitNetworksIntoDB(t *testing.T) {
require.Len(t, returnedSubnets[0].LocalSubnets, 1)
require.Equal(t, "192.0.2.0/24", returnedSubnets[0].Prefix)
require.NotNil(t, returnedSubnets[0].LocalSubnets[0].UserContext)
require.Equal(t, "foo", returnedSubnets[0].Name)

require.Len(t, returnedSubnets[1].LocalSubnets, 1)
require.Equal(t, "192.0.3.0/24", returnedSubnets[1].Prefix)
require.Nil(t, returnedSubnets[1].LocalSubnets[0].UserContext)
require.Empty(t, returnedSubnets[1].Name)

// There should be one shared network.
returnedNetworks, err := GetAllSharedNetworks(db, 4)
Expand All @@ -998,80 +1022,6 @@ func TestCommitNetworksIntoDB(t *testing.T) {
require.Len(t, addedSubnets, 0)
}

// Test the cases when the subnet name is not extracted from the user context.
func TestExtractSubnetNameFromUserContextNegative(t *testing.T) {
t.Run("no local subnets", func(t *testing.T) {
// Arrange
subnet := &Subnet{Name: "foo"}

// Act
name := extractSubnetNameFromUserContext(subnet)

// Assert
require.Empty(t, name)
})

t.Run("no user context", func(t *testing.T) {
// Arrange
subnet := &Subnet{Name: "foo", LocalSubnets: []*LocalSubnet{{
UserContext: nil,
}}}

// Act
name := extractSubnetNameFromUserContext(subnet)

// Assert
require.Empty(t, name)
})

t.Run("no subnet name", func(t *testing.T) {
// Arrange
subnet := &Subnet{Name: "foo", LocalSubnets: []*LocalSubnet{{
UserContext: map[string]any{},
}}}

// Act
name := extractSubnetNameFromUserContext(subnet)

// Assert
require.Empty(t, name)
})
}

// Test the cases when the subnet name is extracted from the user context.
func TestExtractSubnetNameFromUserContextPositive(t *testing.T) {
// Arrange
subnet := &Subnet{Name: "foo", LocalSubnets: []*LocalSubnet{{
UserContext: map[string]any{"subnet-name": "value"},
}}}

// Act
name := extractSubnetNameFromUserContext(subnet)

// Assert
require.Equal(t, "value", name)
}

// Test that the subnet name is extracted from the user context when the subnet
// refers to multiple local subnets.
func TestExtractSubnetNameFromUserContextMultiple(t *testing.T) {
// Arrange
subnet := &Subnet{Name: "foo", LocalSubnets: []*LocalSubnet{
// No user-context.
{UserContext: nil},
// First user-context but with the name key.
{UserContext: map[string]any{"subnet-name": "bar"}},
// Third user-context but with a non-name key.
{UserContext: map[string]any{"label": "baz"}},
}}

// Act
name := extractSubnetNameFromUserContext(subnet)

// Assert
require.Equal(t, "bar", name)
}

// Check if getting subnet family works.
func TestGetSubnetFamily(t *testing.T) {
// create v4 subnet and check its family
Expand Down
2 changes: 0 additions & 2 deletions backend/server/restservice/subnets.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ func (r *RestAPI) convertSubnetToRestAPI(sn *dbmodel.Subnet) *models.Subnet {
PdUtilization: float64(sn.PdUtilization) / 10,
Stats: sn.Stats,
StatsCollectedAt: convertToOptionalDatetime(sn.StatsCollectedAt),
Name: sn.Name,
}

if sn.SharedNetwork != nil {
Expand Down Expand Up @@ -288,7 +287,6 @@ func (r *RestAPI) convertSubnetFromRestAPI(restSubnet *models.Subnet) (*dbmodel.
Prefix: restSubnet.Subnet,
ClientClass: restSubnet.ClientClass,
SharedNetworkID: restSubnet.SharedNetworkID,
Name: restSubnet.Name,
}
hasher := keaconfig.NewHasher()
// Convert local subnet containing associations of the subnet with daemons.
Expand Down
2 changes: 0 additions & 2 deletions backend/server/restservice/subnets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,6 @@ func TestGetSubnet4(t *testing.T) {
ls := subnet.LocalSubnets[0]

// Validate the name.
require.Equal(t, "foo-subnet", subnet.Name)
require.NotNil(t, ls.UserContext)
require.IsType(t, map[string]any(nil), ls.UserContext)
require.EqualValues(t, 42, ls.UserContext.(map[string]any)["answer"])
Expand Down Expand Up @@ -856,7 +855,6 @@ func TestGetSubnet6(t *testing.T) {
ls := subnet.LocalSubnets[0]

// Validate the name.
require.Equal(t, "foo-subnet", subnet.Name)
require.NotNil(t, ls.UserContext)
require.IsType(t, map[string]any(nil), ls.UserContext)
require.EqualValues(t, 42, ls.UserContext.(map[string]any)["answer"])
Expand Down
2 changes: 1 addition & 1 deletion docker/config/agent-kea-premium-one/kea-dhcp4.conf
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
],
"user-context": {
"site": "esperanto",
"name": "valletta"
"subnet-name": "valletta"
}
}]
}
Expand Down
4 changes: 4 additions & 0 deletions docker/config/agent-kea-premium-two/kea-dhcp4.conf
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@
"client-class": "class-03-00",
"relay": {
"ip-addresses": ["172.103.0.200"]
},
"user-context": {
"site": "esperanto",
"subnet-name": "victoria"
}
}]
}
Expand Down
2 changes: 1 addition & 1 deletion hooks/stork-server-ldap
1 change: 0 additions & 1 deletion webui/src/app/subnet-tab/subnet-tab.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<div class="fa fa-project-diagram mr-2"></div>
<div id="tab-title-span" class="word-break-all">
Subnet {{ subnet.subnet }}
<ng-container *ngIf="subnet.name"> named {{ subnet.name }} </ng-container>
<ng-container *ngIf="subnet.sharedNetwork">
in <app-entity-link entity="shared-network" [attrs]="getSharedNetworkAttrs()"></app-entity-link>
</ng-container>
Expand Down
11 changes: 10 additions & 1 deletion webui/src/app/subnets-table/subnets-table.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,16 @@
<app-subnet-bar [subnet]="sn"></app-subnet-bar>
</td>
<td *ngIf="isAnySubnetWithNameVisible">
{{ sn.name }}
<div class="grid" *ngIf="hasAssignedMultipleSubnetNames(sn); else elseSingleSubnetName">
<div class="col-fixed flex flex-column">
<span *ngFor="let lsn of sn.localSubnets">
{{ lsn.userContext?.['subnet-name'] }}
</span>
</div>
</div>
<ng-template #elseSingleSubnetName>
<span>{{ sn.localSubnets?.[0].userContext?.['subnet-name'] }}</span>
</ng-template>
</td>
<td>
<app-human-count [value]="getTotalAddresses(sn)"></app-human-count>
Expand Down
Loading

0 comments on commit 34e3228

Please sign in to comment.