Skip to content

Commit 3f54fba

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #434 from gthiemonge/fix_network_rbac
Disable wildcard RBAC for the provider network
2 parents 61356ac + 57e433b commit 3f54fba

File tree

3 files changed

+144
-9
lines changed

3 files changed

+144
-9
lines changed

‎pkg/octavia/lb_mgmt_network.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external"
2525
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers"
2626
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/provider"
27+
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/rbacpolicies"
2728
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
2829
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules"
2930
"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
@@ -358,6 +359,35 @@ func ensureProvNetwork(client *gophercloud.ServiceClient, netDetails *octaviav1.
358359
return nil, err
359360
}
360361

362+
// Creating an external network adds a RBAC rule to allow all the tenants to use the network.
363+
// Update this rule to make it accessible only by the owner of the network.
364+
// This also fixes the already existing network after an update
365+
listOpts := rbacpolicies.ListOpts{
366+
TenantID: serviceTenantID,
367+
ObjectID: provNet.ID,
368+
TargetTenant: "*",
369+
}
370+
allPages, err := rbacpolicies.List(client, listOpts).AllPages()
371+
if err != nil {
372+
return nil, err
373+
}
374+
375+
allRBACPolicies, err := rbacpolicies.ExtractRBACPolicies(allPages)
376+
if err != nil {
377+
return nil, err
378+
}
379+
380+
for _, rbacpolicy := range allRBACPolicies {
381+
updateOpts := rbacpolicies.UpdateOpts{
382+
TargetTenant: serviceTenantID,
383+
}
384+
385+
_, err := rbacpolicies.Update(client, rbacpolicy.ID, updateOpts).Extract()
386+
if err != nil {
387+
log.Error(err, fmt.Sprintf("Cannot update RBAC policy %s", rbacpolicy.ID))
388+
}
389+
}
390+
361391
return provNet, nil
362392
}
363393

‎tests/functional/neutron_api_fixture.go‎

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,35 @@ import (
2525

2626
"github.com/go-logr/logr"
2727
"github.com/google/uuid"
28+
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external"
2829
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers"
2930
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/quotas"
31+
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/rbacpolicies"
3032
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
3133
"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
3234
"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
3335
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
3436

3537
api "github.com/openstack-k8s-operators/lib-common/modules/test/apis"
38+
"github.com/openstack-k8s-operators/octavia-operator/pkg/octavia"
3639
)
3740

41+
type Network struct {
42+
networks.Network
43+
external.NetworkExternalExt
44+
}
45+
3846
type NeutronAPIFixture struct {
3947
api.APIFixture
4048
Quotas map[string]quotas.Quota
4149
DefaultQuota quotas.Quota
42-
Networks map[string]networks.Network
50+
Networks map[string]Network
4351
Subnets map[string]subnets.Subnet
4452
SecGroups map[string]groups.SecGroup
4553
Ports map[string]ports.Port
4654
Routers map[string]routers.Router
4755
InterfaceInfos map[string]routers.InterfaceInfo
56+
RBACs map[string]rbacpolicies.RBACPolicy
4857
}
4958

5059
func (f *NeutronAPIFixture) registerHandler(handler api.Handler) {
@@ -63,6 +72,8 @@ func (f *NeutronAPIFixture) Setup() {
6372
f.registerHandler(api.Handler{Pattern: "/v2.0/routers/", Func: f.routerHandler})
6473
f.registerHandler(api.Handler{Pattern: "/v2.0/routers", Func: f.routerHandler})
6574
f.registerHandler(api.Handler{Pattern: "/v2.0/quotas/", Func: f.quotasHandler})
75+
f.registerHandler(api.Handler{Pattern: "/v2.0/rbac-policies/", Func: f.rbacHandler})
76+
f.registerHandler(api.Handler{Pattern: "/v2.0/rbac-policies", Func: f.rbacHandler})
6677
}
6778

6879
// Network
@@ -83,9 +94,9 @@ func (f *NeutronAPIFixture) getNetwork(w http.ResponseWriter, r *http.Request) {
8394
items := strings.Split(r.URL.Path, "/")
8495
if len(items) == 4 {
8596
var n struct {
86-
Networks []networks.Network `json:"networks"`
97+
Networks []Network `json:"networks"`
8798
}
88-
n.Networks = []networks.Network{}
99+
n.Networks = []Network{}
89100
query := r.URL.Query()
90101
name := query["name"]
91102
tenantID := query["tenant_id"]
@@ -118,7 +129,7 @@ func (f *NeutronAPIFixture) postNetwork(w http.ResponseWriter, r *http.Request)
118129
}
119130

120131
var n struct {
121-
Network networks.Network `json:"network"`
132+
Network Network `json:"network"`
122133
}
123134

124135
err = json.Unmarshal(bytes, &n)
@@ -131,6 +142,19 @@ func (f *NeutronAPIFixture) postNetwork(w http.ResponseWriter, r *http.Request)
131142
n.Network.ID = networkID
132143
f.Networks[networkID] = n.Network
133144

145+
// Note(gthiemonge) it seems that router:external is not correctly
146+
// unmarshall by the go code, let's assume that only octavia-provider-net is
147+
// an external network
148+
if n.Network.Name == octavia.LbProvNetName {
149+
rbacID := uuid.New().String()
150+
f.RBACs[rbacID] = rbacpolicies.RBACPolicy{
151+
ID: rbacID,
152+
ObjectID: networkID,
153+
TenantID: n.Network.TenantID,
154+
TargetTenant: "*",
155+
}
156+
}
157+
134158
bytes, err = json.Marshal(&n)
135159
if err != nil {
136160
f.InternalError(err, "Error during marshalling response", w, r)
@@ -624,6 +648,78 @@ func (f *NeutronAPIFixture) putQuotas(w http.ResponseWriter, r *http.Request) {
624648
fmt.Fprint(w, string(bytes))
625649
}
626650

651+
func (f *NeutronAPIFixture) rbacHandler(w http.ResponseWriter, r *http.Request) {
652+
f.LogRequest(r)
653+
switch r.Method {
654+
case "GET":
655+
f.getRBAC(w, r)
656+
case "PUT":
657+
f.putRBAC(w, r)
658+
default:
659+
f.UnexpectedRequest(w, r)
660+
return
661+
}
662+
}
663+
664+
func (f *NeutronAPIFixture) getRBAC(w http.ResponseWriter, r *http.Request) {
665+
items := strings.Split(r.URL.Path, "/")
666+
if len(items) == 4 {
667+
var resp struct {
668+
RBACs []rbacpolicies.RBACPolicy `json:"rbac_policies"`
669+
}
670+
resp.RBACs = []rbacpolicies.RBACPolicy{}
671+
query := r.URL.Query()
672+
objectID := query["object_id"][0]
673+
tenantID := query["tenant_id"][0]
674+
targetTenant := query["target_tenant"][0]
675+
for _, rbac := range f.RBACs {
676+
if objectID == rbac.ObjectID &&
677+
tenantID == rbac.TenantID &&
678+
targetTenant == rbac.TargetTenant {
679+
resp.RBACs = append(resp.RBACs, rbac)
680+
}
681+
}
682+
bytes, err := json.Marshal(&resp)
683+
if err != nil {
684+
f.InternalError(err, "Error during marshalling response", w, r)
685+
return
686+
}
687+
688+
w.Header().Add("Content-Type", "application/json")
689+
w.WriteHeader(200)
690+
fmt.Fprint(w, string(bytes))
691+
}
692+
}
693+
694+
func (f *NeutronAPIFixture) putRBAC(w http.ResponseWriter, r *http.Request) {
695+
items := strings.Split(r.URL.Path, "/")
696+
rbacID := items[len(items)-1]
697+
698+
bytes, err := io.ReadAll(r.Body)
699+
if err != nil {
700+
f.InternalError(err, "Error reading request body", w, r)
701+
return
702+
}
703+
var rbac struct {
704+
RBAC rbacpolicies.RBACPolicy `json:"rbac_policy"`
705+
}
706+
err = json.Unmarshal(bytes, &rbac)
707+
if err != nil {
708+
f.InternalError(err, "Error during unmarshalling request", w, r)
709+
return
710+
}
711+
updatedRBAC := f.RBACs[rbacID]
712+
if rbac.RBAC.TargetTenant != "" {
713+
updatedRBAC.TargetTenant = rbac.RBAC.TargetTenant
714+
}
715+
f.APIFixture.Log.Info(fmt.Sprintf("Set RBAC %s to %+v\n", rbacID, updatedRBAC))
716+
f.RBACs[rbacID] = updatedRBAC
717+
718+
w.Header().Add("Content-Type", "application/json")
719+
w.WriteHeader(200)
720+
fmt.Fprint(w, string(bytes))
721+
}
722+
627723
func NewNeutronAPIFixtureWithServer(log logr.Logger) *NeutronAPIFixture {
628724
server := &api.FakeAPIServer{}
629725
server.Setup(log)
@@ -646,12 +742,13 @@ func AddNeutronAPIFixture(log logr.Logger, server *api.FakeAPIServer) *NeutronAP
646742
SecurityGroupRule: 10,
647743
},
648744
Quotas: map[string]quotas.Quota{},
649-
Networks: map[string]networks.Network{},
745+
Networks: map[string]Network{},
650746
Subnets: map[string]subnets.Subnet{},
651747
SecGroups: map[string]groups.SecGroup{},
652748
Ports: map[string]ports.Port{},
653749
Routers: map[string]routers.Router{},
654750
InterfaceInfos: map[string]routers.InterfaceInfo{},
751+
RBACs: map[string]rbacpolicies.RBACPolicy{},
655752
}
656753
return fixture
657754
}

‎tests/functional/octavia_controller_test.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,19 +538,27 @@ var _ = Describe("Octavia controller", func() {
538538
},
539539
}
540540

541-
resultNetworks := map[string]networks.Network{}
541+
resultNetworks := map[string]Network{}
542542
for _, network := range apiFixtures.Neutron.Networks {
543543
resultNetworks[network.Name] = network
544544
}
545545
Expect(resultNetworks).To(HaveLen(2))
546546
for name, expectedNetwork := range expectedNetworks {
547547
network := resultNetworks[name]
548-
Expect(network).ToNot(Equal(networks.Network{}), "Network %s doesn't appear to exist", name)
548+
Expect(network).ToNot(Equal(Network{}), "Network %s doesn't appear to exist", name)
549549
Expect(network.Description).To(Equal(expectedNetwork.Description))
550550
Expect(network.TenantID).To(Equal(expectedNetwork.TenantID))
551551
Expect(network.AvailabilityZoneHints).To(Equal(expectedNetwork.AvailabilityZoneHints))
552552
}
553553

554+
Expect(apiFixtures.Neutron.RBACs).To(HaveLen(1))
555+
for _, rbacPolicy := range apiFixtures.Neutron.RBACs {
556+
providerNetwork := resultNetworks[octavia.LbProvNetName]
557+
Expect(rbacPolicy.ObjectID).To(Equal(providerNetwork.ID))
558+
Expect(rbacPolicy.TenantID).To(Equal(providerNetwork.TenantID))
559+
Expect(rbacPolicy.TargetTenant).To(Equal(providerNetwork.TenantID))
560+
}
561+
554562
lbMgmtPortAddress := ""
555563
lbMgmtPortID := ""
556564
for _, port := range apiFixtures.Neutron.Ports {
@@ -708,14 +716,14 @@ var _ = Describe("Octavia controller", func() {
708716
},
709717
}
710718

711-
resultNetworks := map[string]networks.Network{}
719+
resultNetworks := map[string]Network{}
712720
for _, network := range apiFixtures.Neutron.Networks {
713721
resultNetworks[network.Name] = network
714722
}
715723
Expect(resultNetworks).To(HaveLen(3))
716724
for name, expectedNetwork := range expectedNetworks {
717725
network := resultNetworks[name]
718-
Expect(network).ToNot(Equal(networks.Network{}), "Network %s doesn't appear to exist", name)
726+
Expect(network).ToNot(Equal(Network{}), "Network %s doesn't appear to exist", name)
719727
Expect(network.Description).To(Equal(expectedNetwork.Description))
720728
Expect(network.TenantID).To(Equal(expectedNetwork.TenantID))
721729
Expect(network.AvailabilityZoneHints).To(Equal(expectedNetwork.AvailabilityZoneHints))

0 commit comments

Comments
 (0)