Skip to content

Commit cf0767e

Browse files
jack-w-shawjack-w-shaw
committed
Remove application-offer firewall reads
This document is never written to, and it's intended functionality was replaced by later work. They can be removed safely The only way it can be written to is the user calling set-firewall-rule with juju-application-offer, which is unsupported
1 parent 7e88a34 commit cf0767e

File tree

8 files changed

+24
-165
lines changed

8 files changed

+24
-165
lines changed

‎apiserver/common/crossmodel/crossmodel.go

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package crossmodel
55

66
import (
77
"fmt"
8-
"net"
98

109
"github.com/juju/errors"
1110
"github.com/juju/loggo"
@@ -16,9 +15,7 @@ import (
1615
"github.com/juju/juju/core/life"
1716
corelogger "github.com/juju/juju/core/logger"
1817
"github.com/juju/juju/core/migration"
19-
"github.com/juju/juju/core/network/firewall"
2018
"github.com/juju/juju/core/status"
21-
"github.com/juju/juju/network"
2219
"github.com/juju/juju/rpc/params"
2320
)
2421

@@ -410,59 +407,10 @@ func PublishIngressNetworkChange(backend Backend, relationTag names.Tag, change
410407
return errors.Trace(err)
411408
}
412409

413-
logger.Debugf("relation %v requires ingress networks %v", rel, change.Networks)
414-
if err := validateIngressNetworks(backend, change.Networks); err != nil {
415-
return errors.Trace(err)
416-
}
417-
418410
_, err = backend.SaveIngressNetworks(rel.Tag().Id(), change.Networks)
419411
return err
420412
}
421413

422-
func validateIngressNetworks(backend Backend, networks []string) error {
423-
if len(networks) == 0 {
424-
return nil
425-
}
426-
427-
// Check that the required ingress is allowed.
428-
rule, err := backend.FirewallRule(firewall.JujuApplicationOfferRule)
429-
if err != nil && !errors.IsNotFound(err) {
430-
return errors.Trace(err)
431-
}
432-
if errors.IsNotFound(err) {
433-
return nil
434-
}
435-
var whitelistCIDRs, requestedCIDRs []*net.IPNet
436-
if err := parseCIDRs(&whitelistCIDRs, rule.WhitelistCIDRs()); err != nil {
437-
return errors.Trace(err)
438-
}
439-
if err := parseCIDRs(&requestedCIDRs, networks); err != nil {
440-
return errors.Trace(err)
441-
}
442-
if len(whitelistCIDRs) > 0 {
443-
for _, n := range requestedCIDRs {
444-
if !network.SubnetInAnyRange(whitelistCIDRs, n) {
445-
return &params.Error{
446-
Code: params.CodeForbidden,
447-
Message: fmt.Sprintf("subnet %v not in firewall whitelist", n),
448-
}
449-
}
450-
}
451-
}
452-
return nil
453-
}
454-
455-
func parseCIDRs(cidrs *[]*net.IPNet, values []string) error {
456-
for _, cidrStr := range values {
457-
if _, ipNet, err := net.ParseCIDR(cidrStr); err != nil {
458-
return err
459-
} else {
460-
*cidrs = append(*cidrs, ipNet)
461-
}
462-
}
463-
return nil
464-
}
465-
466414
type relationGetter interface {
467415
// KeyRelation returns the relation identified by the input key.
468416
KeyRelation(string) (Relation, error)

‎apiserver/common/crossmodel/interface.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212

1313
"github.com/juju/juju/core/crossmodel"
1414
"github.com/juju/juju/core/network"
15-
"github.com/juju/juju/core/network/firewall"
1615
"github.com/juju/juju/core/permission"
1716
"github.com/juju/juju/core/status"
1817
"github.com/juju/juju/state"
@@ -94,9 +93,6 @@ type Backend interface {
9493
// lifecycle of the offer.
9594
WatchOffer(offerName string) state.NotifyWatcher
9695

97-
// FirewallRule returns the firewall rule for the specified service.
98-
FirewallRule(service firewall.WellKnownServiceType) (*state.FirewallRule, error)
99-
10096
// ApplyOperation applies a model operation to the state.
10197
ApplyOperation(op state.ModelOperation) error
10298
}

‎apiserver/common/crossmodel/state.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"github.com/juju/names/v4"
99

1010
"github.com/juju/juju/core/crossmodel"
11-
"github.com/juju/juju/core/network/firewall"
1211
"github.com/juju/juju/state"
1312
)
1413

@@ -199,11 +198,6 @@ func (s stateShim) IngressNetworks(relationKey string) (state.RelationNetworks,
199198
return api.Networks(relationKey)
200199
}
201200

202-
func (s stateShim) FirewallRule(service firewall.WellKnownServiceType) (*state.FirewallRule, error) {
203-
api := state.NewFirewallRules(s.State)
204-
return api.Rule(service)
205-
}
206-
207201
type relationShim struct {
208202
*state.Relation
209203
st *state.State

‎apiserver/facades/controller/crossmodelrelations/crossmodelrelations_test.go

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package crossmodelrelations_test
66
import (
77
"bytes"
88
"context"
9-
"regexp"
109
"time"
1110

1211
"github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery"
@@ -26,7 +25,6 @@ import (
2625
apiservertesting "github.com/juju/juju/apiserver/testing"
2726
"github.com/juju/juju/core/crossmodel"
2827
"github.com/juju/juju/core/life"
29-
corefirewall "github.com/juju/juju/core/network/firewall"
3028
"github.com/juju/juju/core/status"
3129
"github.com/juju/juju/core/watcher"
3230
"github.com/juju/juju/rpc/params"
@@ -350,48 +348,6 @@ func (s *crossmodelRelationsSuite) TestPublishIngressNetworkChanges(c *gc.C) {
350348
})
351349
}
352350

353-
func (s *crossmodelRelationsSuite) TestPublishIngressNetworkChangesRejected(c *gc.C) {
354-
s.st.remoteApplications["db2"] = &mockRemoteApplication{}
355-
s.st.relations["db2:db django:db"] = newMockRelation(1)
356-
s.st.remoteEntities[names.NewApplicationTag("db2")] = "token-db2"
357-
s.st.remoteEntities[names.NewRelationTag("db2:db django:db")] = "token-db2:db django:db"
358-
s.st.offerConnectionsByKey["db2:db django:db"] = &mockOfferConnection{
359-
offerUUID: "hosted-db2-uuid",
360-
sourcemodelUUID: "source-model-uuid",
361-
relationKey: "db2:db django:db",
362-
relationId: 1,
363-
}
364-
mac, err := s.bakery.NewMacaroon(
365-
context.TODO(),
366-
bakery.LatestVersion,
367-
[]checkers.Caveat{
368-
checkers.DeclaredCaveat("source-model-uuid", s.st.ModelUUID()),
369-
checkers.DeclaredCaveat("relation-key", "db2:db django:db"),
370-
checkers.DeclaredCaveat("username", "mary"),
371-
}, bakery.Op{"db2:db django:db", "relate"})
372-
373-
c.Assert(err, jc.ErrorIsNil)
374-
rule := state.NewFirewallRule("", []string{"10.1.1.1/8"})
375-
s.st.firewallRules[corefirewall.JujuApplicationOfferRule] = &rule
376-
results, err := s.api.PublishIngressNetworkChanges(params.IngressNetworksChanges{
377-
Changes: []params.IngressNetworksChangeEvent{
378-
{
379-
ApplicationToken: "token-db2",
380-
RelationToken: "token-db2:db django:db",
381-
Networks: []string{"1.2.3.4/32"},
382-
Macaroons: macaroon.Slice{mac.M()},
383-
},
384-
},
385-
})
386-
c.Assert(err, jc.ErrorIsNil)
387-
err = results.Combine()
388-
c.Assert(err, gc.ErrorMatches, regexp.QuoteMeta("subnet 1.2.3.4/32 not in firewall whitelist"))
389-
s.st.CheckCalls(c, []testing.StubCall{
390-
{"GetRemoteEntity", []interface{}{"token-db2:db django:db"}},
391-
{"KeyRelation", []interface{}{"db2:db django:db"}},
392-
})
393-
}
394-
395351
func (s *crossmodelRelationsSuite) TestWatchEgressAddressesForRelations(c *gc.C) {
396352
s.st.remoteEntities[names.NewRelationTag("db2:db django:db")] = "token-db2:db django:db"
397353
s.st.offerConnectionsByKey["db2:db django:db"] = &mockOfferConnection{

‎apiserver/facades/controller/crossmodelrelations/mock_test.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/juju/juju/apiserver/facades/controller/crossmodelrelations"
2525
"github.com/juju/juju/charmstore"
2626
"github.com/juju/juju/core/crossmodel"
27-
corefirewall "github.com/juju/juju/core/network/firewall"
2827
"github.com/juju/juju/core/status"
2928
"github.com/juju/juju/core/watcher"
3029
"github.com/juju/juju/state"
@@ -54,7 +53,6 @@ type mockState struct {
5453
offerConnections map[int]*mockOfferConnection
5554
offerConnectionsByKey map[string]*mockOfferConnection
5655
remoteEntities map[names.Tag]string
57-
firewallRules map[corefirewall.WellKnownServiceType]*state.FirewallRule
5856
ingressNetworks map[string][]string
5957
migrationActive bool
6058
}
@@ -69,7 +67,6 @@ func newMockState() *mockState {
6967
offerNames: make(map[string]string),
7068
offerConnections: make(map[int]*mockOfferConnection),
7169
offerConnectionsByKey: make(map[string]*mockOfferConnection),
72-
firewallRules: make(map[corefirewall.WellKnownServiceType]*state.FirewallRule),
7370
ingressNetworks: make(map[string][]string),
7471
}
7572
}
@@ -131,13 +128,6 @@ func (st *mockState) AddOfferConnection(arg state.AddOfferConnectionParams) (cro
131128
return oc, nil
132129
}
133130

134-
func (st *mockState) FirewallRule(service corefirewall.WellKnownServiceType) (*state.FirewallRule, error) {
135-
if r, ok := st.firewallRules[service]; ok {
136-
return r, nil
137-
}
138-
return nil, errors.NotFoundf("firewall rule for %v", service)
139-
}
140-
141131
func (st *mockState) SaveIngressNetworks(relationKey string, cidrs []string) (state.RelationNetworks, error) {
142132
st.ingressNetworks[relationKey] = cidrs
143133
return nil, nil

‎apiserver/facades/controller/remoterelations/mocks/remoterelations_mocks.go

Lines changed: 0 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎worker/firewaller/firewaller.go

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ type FirewallerAPI interface {
4949
ControllerAPIInfoForModel(modelUUID string) (*api.Info, error)
5050
MacaroonForRelation(relationKey string) (*macaroon.Macaroon, error)
5151
SetRelationStatus(relationKey string, status relation.Status, message string) error
52-
FirewallRules(applicationNames ...string) ([]params.FirewallRule, error)
5352
AllSpaceInfos() (network.SpaceInfos, error)
5453
WatchSubnets() (watcher.StringsWatcher, error)
5554
}
@@ -943,7 +942,7 @@ func (fw *Firewaller) ingressRulesForExposedMachineUnit(machine *machineData, un
943942
}
944943

945944
// TODO(wallyworld) - consider making this configurable.
946-
const maxAllowedCIDRS = 20
945+
const maxAllowedCIDRS = 30
947946

948947
func (fw *Firewaller) updateForRemoteRelationIngress(appTag names.ApplicationTag) (set.Strings, error) {
949948
fw.logger.Debugf("finding egress rules for %v", appTag)
@@ -973,26 +972,9 @@ func (fw *Firewaller) updateForRemoteRelationIngress(appTag names.ApplicationTag
973972
cidrs = set.NewStrings(merged...)
974973
}
975974

976-
// If there's still too many after merging, look for any firewall whitelist.
975+
// If there's still too many after merging, fall back to making public.
977976
if cidrs.Size() > maxAllowedCIDRS {
978-
cidrs = make(set.Strings)
979-
rules, err := fw.firewallerApi.FirewallRules("juju-application-offer")
980-
if err != nil {
981-
return nil, errors.Trace(err)
982-
}
983-
if len(rules) > 0 {
984-
rule := rules[0]
985-
if len(rule.WhitelistCIDRS) > 0 {
986-
for _, cidr := range rule.WhitelistCIDRS {
987-
cidrs.Add(cidr)
988-
}
989-
}
990-
}
991-
// No relevant firewall rule exists, so go public.
992-
if cidrs.Size() == 0 {
993-
cidrs.Add(firewall.AllNetworksIPV4CIDR)
994-
cidrs.Add(firewall.AllNetworksIPV6CIDR)
995-
}
977+
cidrs = set.NewStrings(firewall.AllNetworksIPV4CIDR, firewall.AllNetworksIPV6CIDR)
996978
}
997979

998980
return cidrs, nil

‎worker/firewaller/firewaller_test.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,23 +1281,12 @@ func (s *InstanceModeSuite) TestRemoteRelationProviderRoleOffering(c *gc.C) {
12811281

12821282
func (s *InstanceModeSuite) TestRemoteRelationIngressFallbackToPublic(c *gc.C) {
12831283
var ingress []string
1284-
for i := 1; i < 30; i++ {
1284+
for i := 1; i < 40; i++ {
12851285
ingress = append(ingress, fmt.Sprintf("10.%d.0.1/32", i))
12861286
}
12871287
s.assertIngressCidrs(c, ingress, []string{"0.0.0.0/0", "::/0"})
12881288
}
12891289

1290-
func (s *InstanceModeSuite) TestRemoteRelationIngressFallbackToWhitelist(c *gc.C) {
1291-
fwRules := state.NewFirewallRules(s.State)
1292-
err := fwRules.Save(state.NewFirewallRule(firewall.JujuApplicationOfferRule, []string{"192.168.1.0/16"}))
1293-
c.Assert(err, jc.ErrorIsNil)
1294-
var ingress []string
1295-
for i := 1; i < 30; i++ {
1296-
ingress = append(ingress, fmt.Sprintf("10.%d.0.1/32", i))
1297-
}
1298-
s.assertIngressCidrs(c, ingress, []string{"192.168.1.0/16"})
1299-
}
1300-
13011290
func (s *InstanceModeSuite) TestRemoteRelationIngressMergesCIDRS(c *gc.C) {
13021291
ingress := []string{
13031292
"192.0.1.254/31",
@@ -1321,6 +1310,16 @@ func (s *InstanceModeSuite) TestRemoteRelationIngressMergesCIDRS(c *gc.C) {
13211310
"192.0.4.0/28",
13221311
"192.0.5.0/28",
13231312
"192.0.6.0/28",
1313+
"192.0.7.0/28",
1314+
"192.0.8.0/28",
1315+
"192.0.9.0/28",
1316+
"192.0.10.0/28",
1317+
"192.0.11.0/28",
1318+
"192.0.12.0/28",
1319+
"192.0.13.0/28",
1320+
"192.0.14.0/28",
1321+
"192.0.15.0/28",
1322+
"192.0.16.0/28",
13241323
}
13251324
expected := []string{
13261325
"192.0.1.254/31",
@@ -1329,6 +1328,16 @@ func (s *InstanceModeSuite) TestRemoteRelationIngressMergesCIDRS(c *gc.C) {
13291328
"192.0.4.0/28",
13301329
"192.0.5.0/28",
13311330
"192.0.6.0/28",
1331+
"192.0.7.0/28",
1332+
"192.0.8.0/28",
1333+
"192.0.9.0/28",
1334+
"192.0.10.0/28",
1335+
"192.0.11.0/28",
1336+
"192.0.12.0/28",
1337+
"192.0.13.0/28",
1338+
"192.0.14.0/28",
1339+
"192.0.15.0/28",
1340+
"192.0.16.0/28",
13321341
}
13331342
s.assertIngressCidrs(c, ingress, expected)
13341343
}

0 commit comments

Comments
 (0)