Skip to content

Commit

Permalink
Related to stakater#440
Browse files Browse the repository at this point in the history
Update `GetURL` method in `IngressWrapper` class to check for protocol annotation and use specified protocol if present

* Add `tryGetStatusHost` method to read ingress host from status field
* Update `GetURL` method to use `tryGetStatusHost` method to get host from status field if not found in spec
* Add unit tests for `tryGetStatusHost` method and update existing tests for `GetURL` method
* Add helper functions to create ingress objects with status fields populated for testing
* Add `DefaultProtocol` field in `Config` struct
* Update `getURL` function to include logic for reading ingress host from status field
* Modify `CustomApp` struct to include `Protocol` field and update `convertCustomAppsToForecastleApps` function to use specified protocol

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/stakater/Forecastle/pull/446?shareId=XXXX-XXXX-XXXX-XXXX).
  • Loading branch information
aslafy-z committed Dec 20, 2024
1 parent ce54393 commit b85c3b4
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 36 deletions.
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Config struct {
InstanceName string `yaml:"instanceName" json:"instanceName"`
CustomApps []CustomApp `yaml:"customApps" json:"customApps"`
CRDEnabled bool `yaml:"crdEnabled" json:"crdEnabled"`
DefaultProtocol string `yaml:"defaultProtocol" json:"defaultProtocol"`
}

// CustomApp struct for specifying apps that are not generated using ingresses
Expand All @@ -24,6 +25,7 @@ type CustomApp struct {
Group string `yaml:"group" json:"group"`
NetworkRestricted bool `yaml:"networkRestricted" json:"networkRestricted"`
Properties map[string]string `yaml:"properties" json:"properties"`
Protocol string `yaml:"protocol" json:"protocol"`
}

// NamespaceSelector struct for selecting namespaces based on labels and names
Expand Down
1 change: 0 additions & 1 deletion pkg/forecastle/crdapps/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
func getURL(clients kube.Clients, forecastleApp v1alpha1.ForecastleApp) (string, error) {
if len(forecastleApp.Spec.URL) == 0 {
return discoverURLFromRefs(clients, forecastleApp)

}
return forecastleApp.Spec.URL, nil
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/forecastle/customapps/customapps.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ func (al *List) Get() ([]forecastle.App, error) {

func convertCustomAppsToForecastleApps(customApps []config.CustomApp) (apps []forecastle.App) {
for _, customApp := range customApps {
protocol := customApp.Protocol
if protocol == "" {
protocol = "http"
}
apps = append(apps, forecastle.App{
Name: customApp.Name,
URL: customApp.URL,
URL: protocol + "://" + customApp.URL,
Icon: customApp.Icon,
Group: customApp.Group,
DiscoverySource: forecastle.Config,
Expand Down
75 changes: 49 additions & 26 deletions pkg/kube/wrappers/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,56 +72,79 @@ func (iw *IngressWrapper) GetURL() string {
return parsedURL.String()
}

if !iw.rulesExist() {
logger.Warn("No rules exist in ingress: ", iw.ingress.GetName())
return ""
}

var url string

if host, exists := iw.tryGetTLSHost(); exists { // Get TLS Host if it exists
url = host
protocol := iw.GetAnnotationValue("forecastle.stakater.com/protocol")
if protocol == "" {
if iw.supportsTLS() {
protocol = "https"
} else {
protocol = "http"
}
}

if host, exists := iw.tryGetTLSHost(); exists { // Get TLS Host if defined
url = protocol + "://" + host
} else if host, exists := iw.tryGetRuleHost(); exists { // Fallback to normal host if defined
url = protocol + "://" + host
} else if host, exists := iw.tryGetStatusHost(); exists { // Fallback to status host if defined
url = protocol + "://" + host
} else {
url = iw.getHost() // Fallback for normal Host
logger.Warn("Unable to infer host for ingress: ", iw.ingress.GetName())
return ""
}

// Append port + ingressSubPath
// Append path if defined
url += iw.getIngressSubPath()

return url
}

func (iw *IngressWrapper) rulesExist() bool {
if iw.ingress.Spec.Rules != nil && len(iw.ingress.Spec.Rules) > 0 {
return true
}
return false
func (iw *IngressWrapper) supportsTLS() bool {
return len(iw.ingress.Spec.TLS) > 0
}

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

return "", false
}

func (iw *IngressWrapper) supportsTLS() bool {
if iw.ingress.Spec.TLS != nil && len(iw.ingress.Spec.TLS) > 0 {
return true
func (iw *IngressWrapper) rulesExist() bool {
return len(iw.ingress.Spec.Rules) > 0
}

func (iw *IngressWrapper) tryGetRuleHost() (string, bool) {
if iw.rulesExist() && iw.ingress.Spec.Rules[0].Host != "" {
return iw.ingress.Spec.Rules[0].Host, true
}
return false
return "", false
}

func (iw *IngressWrapper) getHost() string {
return "http://" + iw.ingress.Spec.Rules[0].Host
func (iw *IngressWrapper) statusLoadBalancerExist() bool {
return len(iw.ingress.Status.LoadBalancer.Ingress) > 0
}

func (iw *IngressWrapper) tryGetStatusHost() (string, bool) {
if iw.statusLoadBalancerExist() {
ingressStatus := iw.ingress.Status.LoadBalancer.Ingress[0]
if ingressStatus.Hostname != "" {
return ingressStatus.Hostname, true
} else if ingressStatus.IP != "" {
return ingressStatus.IP, true
}
}
return "", false
}

func (iw *IngressWrapper) getIngressSubPath() string {
rule := iw.ingress.Spec.Rules[0]
if rule.HTTP != nil {
if rule.HTTP.Paths != nil && len(rule.HTTP.Paths) > 0 {
return rule.HTTP.Paths[0].Path
if iw.rulesExist() {
rule := iw.ingress.Spec.Rules[0]
if rule.HTTP != nil {
if len(rule.HTTP.Paths) > 0 {
return rule.HTTP.Paths[0].Path
}
}
}
return ""
Expand Down
95 changes: 87 additions & 8 deletions pkg/kube/wrappers/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ func TestIngressWrapper_GetURL(t *testing.T) {
{
name: "IngressWithTLSHostButNoHost",
fields: fields{
ingress: testutil.CreateIngressWithTLSHost("someIngress2", "https://google.com"),
ingress: testutil.CreateIngressWithTLSHost("someIngress2", "google.com"),
},
want: "",
want: "https://google.com",
},
{
name: "IngressWithTLSHostAndNormalHost",
Expand Down Expand Up @@ -199,6 +199,27 @@ func TestIngressWrapper_GetURL(t *testing.T) {
},
want: "",
},
{
name: "IngressWithStatusHostnameHostButNoHostNorTLSHost",
fields: fields{
ingress: testutil.CreateIngressWithStatusHostnameHost("someIngress2", "google.com"),
},
want: "http://google.com",
},
{
name: "IngressWithStatusIPHostButNoHostNorTLSHost",
fields: fields{
ingress: testutil.CreateIngressWithStatusIPHost("someIngress2", "1.1.1.1"),
},
want: "http://1.1.1.1",
},
{
name: "IngressWithProtocolAnnotation",
fields: fields{
ingress: testutil.AddAnnotationToIngress(testutil.CreateIngressWithHost("someIngress1", "google.com"), "forecastle.stakater.com/protocol", "https"),
},
want: "https://google.com",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -271,7 +292,7 @@ func TestIngressWrapper_tryGetTLSHost(t *testing.T) {
fields: fields{
ingress: testutil.CreateIngressWithHostAndTLSHost("someIngress", "google.com", "google.com"),
},
want: "https://google.com",
want: "google.com",
want1: true,
},
{
Expand Down Expand Up @@ -335,37 +356,95 @@ func TestIngressWrapper_supportsTLS(t *testing.T) {
}
}

func TestIngressWrapper_getHost(t *testing.T) {
func TestIngressWrapper_tryGetRuleHost(t *testing.T) {
type fields struct {
ingress *v1.Ingress
}
tests := []struct {
name string
fields fields
want string
want1 bool
}{
{
name: "IngressWithEmptyHost",
fields: fields{
ingress: testutil.CreateIngressWithHost("someIngress", ""),
},
want: "http://",
want: "",
want1: false,
},
{
name: "IngressWithCorrectHost",
fields: fields{
ingress: testutil.CreateIngressWithHost("someIngress", "google.com"),
},
want: "http://google.com",
want: "google.com",
want1: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
iw := &IngressWrapper{
ingress: tt.fields.ingress,
}
got, got1 := iw.tryGetRuleHost()
if got != tt.want {
t.Errorf("IngressWrapper.tryGetRuleHost() got = %v, want %v", got, tt.want)
}
if got1 != tt.want1 {
t.Errorf("IngressWrapper.tryGetRuleHost() got1 = %v, want %v", got1, tt.want1)
}
})
}
}

func TestIngressWrapper_tryGetStatusHost(t *testing.T) {
type fields struct {
ingress *v1.Ingress
}
tests := []struct {
name string
fields fields
want string
want1 bool
}{
{
name: "IngressWithEmptyStatusHost",
fields: fields{
ingress: testutil.CreateIngressWithHost("someIngress", ""),
},
want: "",
want1: false,
},
{
name: "IngressWithCorrectHostnameStatusHost",
fields: fields{
ingress: testutil.CreateIngressWithStatusHostnameHost("someIngress", "google.com"),
},
want: "google.com",
want1: true,
},
{
name: "IngressWithCorrectIPStatusHost",
fields: fields{
ingress: testutil.CreateIngressWithStatusIPHost("someIngress", "1.1.1.1"),
},
want: "1.1.1.1",
want1: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
iw := &IngressWrapper{
ingress: tt.fields.ingress,
}
if got := iw.getHost(); got != tt.want {
t.Errorf("IngressWrapper.getHost() = %v, want %v", got, tt.want)
got, got1 := iw.tryGetStatusHost()
if got != tt.want {
t.Errorf("IngressWrapper.tryGetStatusHost() got = %v, want %v", got, tt.want)
}
if got1 != tt.want1 {
t.Errorf("IngressWrapper.tryGetStatusHost() got1 = %v, want %v", got1, tt.want1)
}
})
}
Expand Down
33 changes: 33 additions & 0 deletions pkg/testutil/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,28 @@ func CreateIngressWithHostAndEmptyTLSHost(name string, host string) *v1.Ingress
return ingress
}

func CreateIngressWithStatusHostnameHost(name string, host string) *v1.Ingress {
ingress := CreateIngress(name)
ingress.Status.LoadBalancer.Ingress = []v1.IngressLoadBalancerIngress{
{
Hostname: host,
},
}

return ingress
}

func CreateIngressWithStatusIPHost(name string, host string) *v1.Ingress {
ingress := CreateIngress(name)
ingress.Status.LoadBalancer.Ingress = []v1.IngressLoadBalancerIngress{
{
IP: host,
},
}

return ingress
}

func CreateForecastleApp(name string, url string, group string, icon string) *v1alpha1.ForecastleApp {
return &v1alpha1.ForecastleApp{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -157,3 +179,14 @@ func CreateForecastleAppWithURLFromIngress(name string, group string, icon strin

return forecastleApp
}

func CreateIngressWithStatusFields(name string, hostname string, ip string) *v1.Ingress {
ingress := CreateIngress(name)
ingress.Status.LoadBalancer.Ingress = []v1.IngressLoadBalancerIngress{
{
Hostname: hostname,
IP: ip,
},
}
return ingress
}

0 comments on commit b85c3b4

Please sign in to comment.