Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added verification to avoid creating duplicate ingress if an unmanaged ingress already exists #633

Merged
merged 1 commit into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## [NEXT_RELEASE]
### Added
- Verification to avoid ingress duplication when an unmanaged ingress exists

### Changed
- Default nginx side-car image to allow support to lua scripts
- Ingress spec will be the same for vhost and static IP
Expand Down
1 change: 1 addition & 0 deletions helm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Parameter | Description | Default
`useMinio` | If true, use minio instead of s3. | `false`
`rbac.enabled` | If true, this configure teresa deployment to use rbac, for now it will use the `cluster-admin` role | `false`
`apps.ingress` | If true, teresa will create a ingress when expose the app | `false`
`apps.check_annother_ingress` | If true, Teresa will halt exposing an app if an ingress with a different name than the app name exists | `true`
`apps.service_type` | The type used to create the app server | `LoadBalancer`
`apps.ingress_class` | The ingress class to be used | ``
`slugstore.image`| The image to be used for the slugstore | `""`
Expand Down
2 changes: 2 additions & 0 deletions helm/chart/teresa/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ spec:
value: {{ .Values.apps.service_type }}
- name: TERESA_DEPLOY_INGRESS_CLASS
value: {{ .Values.apps.ingress_class }}
- name: TERESA_K8S_CHECK_ANOTHER_INGRESS
value: {{ .Values.apps.check_another_ingress | quote}}
{{- if .Values.apps.dnsconfig_ndots }}
- name: TERESA_DEPLOY_DNSCONFIG_NDOTS
value: {{ .Values.apps.dnsconfig_ndots }}
Expand Down
1 change: 1 addition & 0 deletions helm/chart/teresa/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ apps:
ingress: false
service_type: LoadBalancer
ingress_class:
check_another_ingress: true
slugstore:
image:
slugbuilder:
Expand Down
60 changes: 49 additions & 11 deletions pkg/server/k8s/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ const (
)

type Client struct {
conf *restclient.Config
podRunTimeout time.Duration
ingress bool
fake kubernetes.Interface
testing bool
conf *restclient.Config
podRunTimeout time.Duration
ingress bool
fake kubernetes.Interface
testing bool
checkAnotherIngress bool
}

func (k *Client) buildClient() (kubernetes.Interface, error) {
Expand Down Expand Up @@ -623,6 +624,28 @@ func (k *Client) HasIngress(namespace, appName string) (bool, error) {
return true, nil
}

// HasAnotherIngress will check if there is another ingress than the one with the same name as the app
func (k *Client) HasAnotherIngress(namespace, appName string) (bool, error) {
kc, err := k.buildClient()
if err != nil {
return false, err
}
opts := metav1.ListOptions{
FieldSelector: fmt.Sprintf("metadata.name!=%s", appName),
}
ingList, err := kc.ExtensionsV1beta1().
Ingresses(namespace).
List(opts)

if err != nil {
if k.IsNotFound(err) {
return false, nil
}
return false, errors.Wrap(err, "get ingress failed")
}
return len(ingList.Items) > 0, nil
}

func (k *Client) createIngress(namespace, appName string, vHosts []string, ingressClass string) error {
kc, err := k.buildClient()
if err != nil {
Expand Down Expand Up @@ -719,7 +742,20 @@ func (k *Client) ExposeDeploy(namespace, appName, svcType, portName string, vHos
if err != nil {
return err
}
if !hasIgs {

hasAnotherIgs := false
if k.checkAnotherIngress {
hasAnotherIgs, err = k.HasAnotherIngress(namespace, appName)
if err != nil {
return err
}

if hasAnotherIgs {
fmt.Fprintln(w, "There is another ingress on the app that is not managed by Teresa.")
}
}

if !hasIgs && !hasAnotherIgs {
if len(vHosts) == 0 {
fmt.Fprintln(w, "To expose a Ingress please provide at last one vHost")
return nil
Expand Down Expand Up @@ -1656,8 +1692,9 @@ func newInClusterK8sClient(conf *Config) (*Client, error) {
return nil, err
}
return &Client{
conf: k8sConf,
ingress: conf.Ingress,
conf: k8sConf,
ingress: conf.Ingress,
checkAnotherIngress: conf.CheckAnotherIngress,
}, nil
}

Expand All @@ -1667,8 +1704,9 @@ func newOutOfClusterK8sClient(conf *Config) (*Client, error) {
return nil, err
}
return &Client{
conf: k8sConf,
podRunTimeout: conf.PodRunTimeout,
ingress: conf.Ingress,
conf: k8sConf,
podRunTimeout: conf.PodRunTimeout,
ingress: conf.Ingress,
checkAnotherIngress: conf.CheckAnotherIngress,
}, nil
}
20 changes: 20 additions & 0 deletions pkg/server/k8s/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,23 @@ func TestClientCreateNamespace(t *testing.T) {
t.Errorf("got %s; want test", an)
}
}

func TestHasAnotherIngress(t *testing.T) {
cli := &Client{testing: true}

if err := cli.createIngress("test", "ingress1", []string{"xpto.com"}, "nginx"); err != nil {
t.Fatal("got unexpected error:", err)
}

if err := cli.createIngress("test", "ingress2", []string{"xpto.com"}, "nginx"); err != nil {
t.Fatal("got unexpected error:", err)
}
rs, err := cli.HasAnotherIngress("test", "test")
if err != nil {
t.Fatal("got unexpected error:", err)
}

if rs != false {
t.Errorf("got true; want false")
}
}
2 changes: 1 addition & 1 deletion pkg/server/k8s/converters.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func ingressSpec(namespace, name string, vHosts []string) *k8s_extensions.Ingres
HTTP: &k8s_extensions.HTTPIngressRuleValue{
Paths: []k8s_extensions.HTTPIngressPath{
{
Path: "/",
Path: "/*",
Backend: *backend,
},
},
Expand Down
7 changes: 4 additions & 3 deletions pkg/server/k8s/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ var validServiceTypes = map[k8sv1.ServiceType]bool{
}

type Config struct {
ConfigFile string `split_words:"true"`
PodRunTimeout time.Duration `split_words:"true" default:"30m"`
Ingress bool `split_words:"true" default:"false"`
ConfigFile string `split_words:"true"`
PodRunTimeout time.Duration `split_words:"true" default:"30m"`
Ingress bool `split_words:"true" default:"false"`
CheckAnotherIngress bool `split_words:"true" default:"true"`
}

func New(conf *Config) (*Client, error) {
Expand Down