Hi folks, thanks for this project! For reasons we

Hi <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="

forecastle should crash/fail when run into panic about forecastle HOT 1 CLOSED

1337andre commented on September 23, 2024

forecastle should crash/fail when run into panic

from forecastle.

Comments (1)

hassaanakram commented on September 23, 2024

Hi @karl-johan-grahn
I'd like to work on this enhancement. I have gone through the relevant code and my analysis is as follows.

Cause of this panic
In

Forecastle/pkg/kube/wrappers/ingress.go

Line 103 in 53888ad

return "https://" + iw.ingress.Spec.TLS[0].Hosts[0], true

the Hosts slice is empty and therefore attempting to access Hosts[0] causes the runtime panic.

What can be done to make this access safe
Add a new method getTLSHosts on the IngressWrapper:

func (iw *IngressWrapper) getTLSHosts() []string {
	if iw.supportsTLS() {
		return iw.ingress.Spec.TLS[0].Hosts
	}
	return []string{}
}

Similarly change the method tryGetTLSHost to:

func (iw *IngressWrapper) tryGetTLSHost() (string, bool) {
	if iw.supportsTLS() && len(iw.getTLSHosts()) > 0 {
		return "https://" + iw.ingress.Spec.TLS[0].Hosts[0], true
	}

	return "", false
}

Test cases
In https://github.com/stakater/Forecastle/blob/master/pkg/testutil/kube.go add the following function to simulate a case where TLS hosts is empty:

func CreateIngressWithHostAndEmptyTLSHost(name string, host string) *v1.Ingress {
	ingress := CreateIngressWithHost(name, host)
	ingress.Spec.TLS = []v1.IngressTLS{
		{
			Hosts: []string{},
		},
	}

	return ingress
}

Add the following test case in

Forecastle/pkg/kube/wrappers/ingress_test.go

Line 251 in 53888ad

func TestIngressWrapper_tryGetTLSHost(t *testing.T) {

{
			name: "IngressWithTLSAndNoHosts",
			fields: fields{
				ingress: testutil.CreateIngressWithHostAndEmptyTLSHost("someIngress", "google.com"),
			},
			want:  "",
			want1: false,
		},

Let me know what do you think about this.

from forecastle.

Recommend Projects