Skip to content

Commit

Permalink
Adds rewrite-target annotation, tests for Nginx Annotations… (#637)
Browse files Browse the repository at this point in the history
* Adds rewrite-target annotation, tests for Nginx Annotations and attempts to singleton kubernetes fake client
  • Loading branch information
retpolanne authored Jun 17, 2020
1 parent 31c8d09 commit 5393e78
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 28 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## [NEXT_RELEASE]
### Added
- Verification to avoid ingress duplication when an unmanaged ingress exists
- Adds rewrite-target annotation to createIngress if ingressClass is nginx

### Changed
- Default nginx side-car image to allow support to lua scripts
- Ingress spec will be the same for vhost and static IP
- Checks if app already has ingress before trying to reserve a static IP
- Moves ingress annotations to deploy's exposeApp

### Fixed
- Don't create an Ingress for apps without vHosts
Expand Down
24 changes: 20 additions & 4 deletions pkg/server/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type K8sOperations interface {
ContainerExplicitEnvVars(namespace, deployName, containerName string) ([]*app.EnvVar, error)
WatchDeploy(namespace, deployName string) error
HasIngress(namespace, appName string) (bool, error)
SetIngressAnnotations(namespace, ingName string, annotations map[string]string) error
}

type CloudProviderOperations interface {
Expand Down Expand Up @@ -243,10 +244,25 @@ func (ops *DeployOperations) exposeApp(a *app.App, w io.Writer) error {
if err != nil {
return err
}
if a.ReserveStaticIp && hasIngress {
addressName := fmt.Sprintf("%s-ingress", a.Name)
if err := ops.cOps.CreateOrUpdateStaticIp(a.Name, addressName); err != nil {
return err
if hasIngress {
if a.ReserveStaticIp {
addressName := fmt.Sprintf("%s-ingress", a.Name)
if err := ops.cOps.CreateOrUpdateStaticIp(a.Name, addressName); err != nil {
return err
}
}
if ops.opts.IngressClass != "" {
annotations := make(map[string]string)
annotations["kubernetes.io/ingress.class"] = ops.opts.IngressClass
if ops.opts.IngressClass == "nginx" {
annotations["nginx.ingress.kubernetes.io/rewrite-target"] = "/"
}
if err = ops.k8s.SetIngressAnnotations(
a.Name, a.Name,
annotations,
); err != nil {
return errors.Wrap(err, "add ingress annotations failed")
}
}
}
return nil // already exposed
Expand Down
81 changes: 69 additions & 12 deletions pkg/server/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ import (
)

type fakeK8sOperations struct {
lastDeploySpec *spec.Deploy
lastCronJobSpec *spec.CronJob
createDeployReturn error
createCronJobReturn error
hasSrvErr error
exposeDeployWasCalled bool
hasIngress bool
replicaSetListByLabelErr error
createConfigMapWasCalled bool
deleteConfigMapWasCalled bool
containerExplicitEnvVarsErr error
containerExplicitEnvVarsValue []*app.EnvVar
lastDeploySpec *spec.Deploy
lastCronJobSpec *spec.CronJob
createDeployReturn error
createCronJobReturn error
hasSrvErr error
exposeDeployWasCalled bool
setIngressAnnotationsWasCalled bool
setIngressAnnotationsIngClass string
setIngressAnnotationsRewriteTarget string
hasIngress bool
replicaSetListByLabelErr error
createConfigMapWasCalled bool
deleteConfigMapWasCalled bool
containerExplicitEnvVarsErr error
containerExplicitEnvVarsValue []*app.EnvVar
}

func (f *fakeK8sOperations) CreateOrUpdateConfigMap(namespace, name string, data map[string]string) error {
Expand Down Expand Up @@ -72,6 +75,13 @@ func (f *fakeK8sOperations) HasIngress(namespace, appName string) (bool, error)
return f.hasIngress, nil
}

func (f *fakeK8sOperations) SetIngressAnnotations(namespace, ingName string, annotations map[string]string) error {
f.setIngressAnnotationsWasCalled = true
f.setIngressAnnotationsIngClass = annotations["kubernetes.io/ingress.class"]
f.setIngressAnnotationsRewriteTarget = annotations["nginx.ingress.kubernetes.io/rewrite-target"]
return nil
}

type fakeCloudProviderOperations struct {
createOrUpdateStaticIPWasCalled bool
}
Expand Down Expand Up @@ -522,6 +532,53 @@ func TestCreateCronJobScheduleNotFound(t *testing.T) {
}
}

func TestExposeAppNginxIngress(t *testing.T) {
fakeK8s := &fakeK8sOperations{
hasIngress: true,
}
fakeCops := &fakeCloudProviderOperations{}
ops := NewDeployOperations(
app.NewFakeOperations(),
fakeK8s,
storage.NewFake(),
exec.NewFakeOperations(),
build.NewFakeOperations(),
fakeCops,
&Options{
IngressClass: "nginx",
},
)
deployOperations := ops.(*DeployOperations)
deployOperations.exposeApp(&app.App{
ProcessType: "web",
Name: "test",
}, new(bytes.Buffer))

if !fakeK8s.setIngressAnnotationsWasCalled {
t.Errorf(
"expected setIngressAnnotationsWasCalled to be %v, got %v",
true,
fakeK8s.setIngressAnnotationsWasCalled,
)
}

if fakeK8s.setIngressAnnotationsIngClass != "nginx" {
t.Errorf(
"expected setIngressAnnotationsIngClass to be %v, got %v",
"nginx",
fakeK8s.setIngressAnnotationsIngClass,
)
}

if fakeK8s.setIngressAnnotationsRewriteTarget != "/" {
t.Errorf(
"expected setIngressAnnotationsRewriteTarget to be %v, got %v",
"/",
fakeK8s.setIngressAnnotationsRewriteTarget,
)
}
}

func TestExposeApp(t *testing.T) {
var testCases = []struct {
appProcessType string
Expand Down
12 changes: 3 additions & 9 deletions pkg/server/k8s/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ type Client struct {

func (k *Client) buildClient() (kubernetes.Interface, error) {
if k.testing {
k.fake = fake.NewSimpleClientset()
if k.fake == nil {
k.fake = fake.NewSimpleClientset()
}
return k.fake, nil
}
c, err := kubernetes.NewForConfig(k.conf)
Expand Down Expand Up @@ -656,14 +658,6 @@ func (k *Client) createIngress(namespace, appName string, vHosts []string, ingre
if err != nil {
return errors.Wrap(err, "create ingress failed")
}
if ingressClass != "" {
if err = k.SetIngressAnnotations(
namespace, appName,
map[string]string{"kubernetes.io/ingress.class": ingressClass},
); err != nil {
return errors.Wrap(err, "create ingress failed")
}
}
return nil
}

Expand Down
16 changes: 13 additions & 3 deletions pkg/server/k8s/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func setupK8sCli(t *testing.T, cli *Client) (func(t *testing.T, cli *Client), error) {
return func(t *testing.T, cli *Client) {
cli.fake = nil
}, nil
}

func TestAddVolumeMountOfSecrets(t *testing.T) {
var testCases = []struct {
vols []k8sv1.VolumeMount
Expand Down Expand Up @@ -344,6 +350,8 @@ func TestRemoveSecretVols(t *testing.T) {
func TestClientCreateNamespace(t *testing.T) {
a := &app.App{Name: "test"}
cli := &Client{testing: true}
teardownCli, err := setupK8sCli(t, cli)
defer teardownCli(t, cli)

if err := cli.CreateNamespace(a, "test"); err != nil {
t.Fatal("got unexpected error:", err)
Expand All @@ -361,6 +369,8 @@ func TestClientCreateNamespace(t *testing.T) {

func TestHasAnotherIngress(t *testing.T) {
cli := &Client{testing: true}
teardownCli, err := setupK8sCli(t, cli)
defer teardownCli(t, cli)

if err := cli.createIngress("test", "ingress1", []string{"xpto.com"}, "nginx"); err != nil {
t.Fatal("got unexpected error:", err)
Expand All @@ -369,12 +379,12 @@ func TestHasAnotherIngress(t *testing.T) {
if err := cli.createIngress("test", "ingress2", []string{"xpto.com"}, "nginx"); err != nil {
t.Fatal("got unexpected error:", err)
}
rs, err := cli.HasAnotherIngress("test", "test")
hasAnotherIngress, err := cli.HasAnotherIngress("test", "test")
if err != nil {
t.Fatal("got unexpected error:", err)
}

if rs != false {
t.Errorf("got true; want false")
if hasAnotherIngress != true {
t.Errorf("hasAnotherIngress returned false; want true")
}
}

0 comments on commit 5393e78

Please sign in to comment.