Skip to content

Commit

Permalink
verify broker name is in lower case letters (#109)
Browse files Browse the repository at this point in the history 
* implementation + tests for GetBrokerPlatformName interface
* update version for service-broker-proxy
  • Loading branch information
@evyaffe
evyaffe authored Nov 9, 2020
1 parent 1168464 commit 396bda9
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 55 deletions.
21 changes: 15 additions & 6 deletions Gopkg.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Gopkg.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

[[constraint]]
name = "github.com/Peripli/service-broker-proxy"
version = "=0.10.0"
version = "=0.11.9"

[[constraint]]
name = "github.com/onsi/ginkgo"
Expand Down
7 changes: 7 additions & 0 deletions pkg/k8s/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
servicecatalog "github.com/kubernetes-sigs/service-catalog/pkg/svcat/service-catalog"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"strings"
"sync"

"github.com/Peripli/service-broker-proxy/pkg/platform"
Expand Down Expand Up @@ -368,6 +369,12 @@ func (pc *PlatformClient) Fetch(ctx context.Context, r *platform.UpdateServiceBr
return pc.platformAPI.SyncNamespaceServiceBroker(r.Name, pc.targetNamespace, resyncBrokerRetryCount)
}

// GetBrokerPlatformName enforces broker names to be as k8s requires.
// Name will be later prefixed an suffixed by allowed strings, so we only make sure lowercase and replace underscore with hyphen.
func (pc *PlatformClient) GetBrokerPlatformName(name string) string {
return strings.ReplaceAll(strings.ToLower(name), "_", "-")
}

func (pc *PlatformClient) updateBrokerPlatformSecret(name, username, password string) error {
var secretNamespace string
if pc.isClusterScoped() {
Expand Down
105 changes: 57 additions & 48 deletions pkg/k8s/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,17 @@ func TestClient(t *testing.T) {
}

var _ = Describe("Kubernetes Broker Proxy", func() {
var expectedError = errors.New("expected")
var clientConfig *config.ClientConfiguration
var proxySettings *sbproxy.Settings
var settings *config.Settings
var ctx context.Context
var k8sApi *apifakes.FakeKubernetesAPI
const fakeBrokerName = "fake-broker"
const fakeBrokerUrl = "http://fake.broker.url"

var (
expectedError = errors.New("expected")
clientConfig *config.ClientConfiguration
proxySettings *sbproxy.Settings
settings *config.Settings
ctx context.Context
k8sApi *apifakes.FakeKubernetesAPI
)

newDefaultPlatformClient := func() *PlatformClient {
client, err := NewClient(settings)
Expand Down Expand Up @@ -138,8 +143,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

requestBroker := &platform.CreateServiceBrokerRequest{
ID: "id-in-sm",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand All @@ -154,8 +159,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

Expect(err).To(BeNil())
Expect(createdBroker.GUID).To(Equal("1234"))
Expect(createdBroker.Name).To(Equal("fake-broker"))
Expect(createdBroker.BrokerURL).To(Equal("http://fake.broker.url"))
Expect(createdBroker.Name).To(Equal(fakeBrokerName))
Expect(createdBroker.BrokerURL).To(Equal(fakeBrokerUrl))
})
})

Expand Down Expand Up @@ -191,11 +196,10 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.DeleteServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
Name: fakeBrokerName,
}

err := platformClient.DeleteBroker(ctx, requestBroker)

Expect(err).To(BeNil())
})
})
Expand All @@ -211,7 +215,6 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.DeleteServiceBrokerRequest{}

err := platformClient.DeleteBroker(ctx, requestBroker)

Expect(err).To(Equal(errors.New("error deleting clusterservicebroker")))
})
})
Expand All @@ -227,11 +230,11 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
brokers = append(brokers, v1beta1.ClusterServiceBroker{
ObjectMeta: v1.ObjectMeta{
UID: "1234",
Name: "fake-broker",
Name: fakeBrokerName,
},
Spec: v1beta1.ClusterServiceBrokerSpec{
CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{
URL: "http://fake.broker.url",
URL: fakeBrokerUrl,
},
},
})
Expand All @@ -246,8 +249,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
Expect(brokers).ToNot(BeNil())
Expect(len(brokers)).To(Equal(1))
Expect(brokers[0].GUID).To(Equal("1234"))
Expect(brokers[0].Name).To(Equal("fake-broker"))
Expect(brokers[0].BrokerURL).To(Equal("http://fake.broker.url"))
Expect(brokers[0].Name).To(Equal(fakeBrokerName))
Expect(brokers[0].BrokerURL).To(Equal(fakeBrokerUrl))
})
})

Expand Down Expand Up @@ -291,27 +294,26 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
Context("with no error", func() {
It("returns the service broker", func() {
platformClient := newDefaultPlatformClient()
brokerName := "brokerName"

k8sApi.RetrieveClusterServiceBrokerByNameStub = func(name string) (*v1beta1.ClusterServiceBroker, error) {
return &v1beta1.ClusterServiceBroker{
ObjectMeta: v1.ObjectMeta{
UID: "1234",
Name: brokerName,
Name: fakeBrokerName,
},
Spec: v1beta1.ClusterServiceBrokerSpec{
CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{
URL: "http://fake.broker.url",
URL: fakeBrokerUrl,
},
},
}, nil
}

broker, err := platformClient.GetBrokerByName(ctx, brokerName)
broker, err := platformClient.GetBrokerByName(ctx, fakeBrokerName)

Expect(err).To(BeNil())
Expect(broker).ToNot(BeNil())
Expect(broker.Name).To(Equal(brokerName))
Expect(broker.Name).To(Equal(fakeBrokerName))
})
})

Expand Down Expand Up @@ -356,8 +358,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.UpdateServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand All @@ -373,8 +375,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

Expect(err).To(BeNil())
Expect(broker.GUID).To(Equal("1234"))
Expect(broker.Name).To(Equal("fake-broker-updated"))
Expect(broker.BrokerURL).To(Equal("http://fake.broker.url-updated"))
Expect(broker.Name).To(Equal(fakeBrokerName + "-updated"))
Expect(broker.BrokerURL).To(Equal(fakeBrokerUrl + "-updated"))
})
})

Expand Down Expand Up @@ -407,8 +409,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.UpdateServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand Down Expand Up @@ -509,8 +511,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

requestBroker := &platform.CreateServiceBrokerRequest{
ID: "id-in-sm",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand All @@ -525,8 +527,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

Expect(err).To(BeNil())
Expect(createdBroker.GUID).To(Equal("1234"))
Expect(createdBroker.Name).To(Equal("fake-broker"))
Expect(createdBroker.BrokerURL).To(Equal("http://fake.broker.url"))
Expect(createdBroker.Name).To(Equal(fakeBrokerName))
Expect(createdBroker.BrokerURL).To(Equal(fakeBrokerUrl))
})
})

Expand Down Expand Up @@ -562,7 +564,7 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.DeleteServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
Name: fakeBrokerName,
}

err := platformClient.DeleteBroker(ctx, requestBroker)
Expand Down Expand Up @@ -598,11 +600,11 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
brokers = append(brokers, v1beta1.ServiceBroker{
ObjectMeta: v1.ObjectMeta{
UID: "1234",
Name: "fake-broker",
Name: fakeBrokerName,
},
Spec: v1beta1.ServiceBrokerSpec{
CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{
URL: "http://fake.broker.url",
URL: fakeBrokerUrl,
},
},
})
Expand All @@ -617,8 +619,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
Expect(brokers).ToNot(BeNil())
Expect(len(brokers)).To(Equal(1))
Expect(brokers[0].GUID).To(Equal("1234"))
Expect(brokers[0].Name).To(Equal("fake-broker"))
Expect(brokers[0].BrokerURL).To(Equal("http://fake.broker.url"))
Expect(brokers[0].Name).To(Equal(fakeBrokerName))
Expect(brokers[0].BrokerURL).To(Equal(fakeBrokerUrl))
})
})

Expand Down Expand Up @@ -662,27 +664,26 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
Context("with no error", func() {
It("returns the service broker", func() {
platformClient := newDefaultPlatformClient()
brokerName := "brokerName"

k8sApi.RetrieveNamespaceServiceBrokerByNameStub = func(name, namespace string) (*v1beta1.ServiceBroker, error) {
return &v1beta1.ServiceBroker{
ObjectMeta: v1.ObjectMeta{
UID: "1234",
Name: brokerName,
Name: fakeBrokerName,
},
Spec: v1beta1.ServiceBrokerSpec{
CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{
URL: "http://fake.broker.url",
URL: fakeBrokerUrl,
},
},
}, nil
}

broker, err := platformClient.GetBrokerByName(ctx, brokerName)
broker, err := platformClient.GetBrokerByName(ctx, fakeBrokerName)

Expect(err).To(BeNil())
Expect(broker).ToNot(BeNil())
Expect(broker.Name).To(Equal(brokerName))
Expect(broker.Name).To(Equal(fakeBrokerName))
})
})

Expand Down Expand Up @@ -727,8 +728,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.UpdateServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand All @@ -744,8 +745,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {

Expect(err).To(BeNil())
Expect(broker.GUID).To(Equal("1234"))
Expect(broker.Name).To(Equal("fake-broker-updated"))
Expect(broker.BrokerURL).To(Equal("http://fake.broker.url-updated"))
Expect(broker.Name).To(Equal(fakeBrokerName + "-updated"))
Expect(broker.BrokerURL).To(Equal(fakeBrokerUrl + "-updated"))
})
})

Expand Down Expand Up @@ -778,8 +779,8 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
requestBroker := &platform.UpdateServiceBrokerRequest{
ID: "id-in-sm",
GUID: "1234",
Name: "fake-broker",
BrokerURL: "http://fake.broker.url",
Name: fakeBrokerName,
BrokerURL: fakeBrokerUrl,
Username: "admin",
Password: "admin",
}
Expand Down Expand Up @@ -869,4 +870,12 @@ var _ = Describe("Kubernetes Broker Proxy", func() {
})
})

Describe("Platform Broker Name", func() {
It("returns lower case and replaces underscores to hyphens", func() {
brokerNameWithUnderscoreAndCaps := "Fake_Broker-Name_1234"
expectedBrokerName := "fake-broker-name-1234"
platformClient := newDefaultPlatformClient()
Expect(platformClient.GetBrokerPlatformName(brokerNameWithUnderscoreAndCaps)).To(Equal(expectedBrokerName))
})
})
})

0 comments on commit 396bda9

Please sign in to comment.