Skip to content

Commit

Permalink
fix(api): prevent nil dereferencing in GetInstanceInfo method
Browse files Browse the repository at this point in the history
  • Loading branch information
@nettoclaudio
nettoclaudio committed Mar 25, 2020
1 parent a093664 commit 17c911b
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 44 deletions.
77 changes: 33 additions & 44 deletions internal/pkg/rpaas/k8s.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1378,35 +1378,36 @@ func setTeamOwner(instance *v1alpha1.RpaasInstance, team string) {
instance.Spec.PodTemplate.Labels = mergeMap(instance.Spec.PodTemplate.Labels, newLabels)
}

func newInstanceInfo(instance *v1alpha1.RpaasInstance, ingresses []corev1.LoadBalancerIngress) *clientTypes.InstanceInfo {
func newInstanceInfo(instance *v1alpha1.RpaasInstance, routes []Route, ingresses []corev1.LoadBalancerIngress) *clientTypes.InstanceInfo {
info := &clientTypes.InstanceInfo{
Replicas: instance.Spec.Replicas,
Plan: instance.Spec.PlanName,
Autoscale: &clientTypes.Autoscale{
MinReplicas: instance.Spec.Autoscale.MinReplicas,
MaxReplicas: &instance.Spec.Autoscale.MaxReplicas,
CPU: instance.Spec.Autoscale.TargetCPUUtilizationPercentage,
Memory: instance.Spec.Autoscale.TargetMemoryUtilizationPercentage,
},
Binds: instance.Spec.Binds,
Name: instance.ObjectMeta.Name,
Name: instance.ObjectMeta.Name,
Description: instance.Annotations[labelKey("description")],
Team: instance.Annotations[labelKey("team-owner")],
Tags: strings.Split(instance.Annotations[labelKey("tags")], ","),
Replicas: instance.Spec.Replicas,
Plan: instance.Spec.PlanName,
Binds: instance.Spec.Binds,
}

for _, route := range instance.Spec.Locations {
info.Routes = append(info.Routes, clientTypes.Route{
Path: route.Path,
Destination: route.Destination,
})
autoscale := instance.Spec.Autoscale
if autoscale != nil {
info.Autoscale = &clientTypes.Autoscale{
MinReplicas: autoscale.MinReplicas,
MaxReplicas: &autoscale.MaxReplicas,
CPU: autoscale.TargetCPUUtilizationPercentage,
Memory: autoscale.TargetMemoryUtilizationPercentage,
}
}

if desc, ok := instance.ObjectMeta.Annotations["description"]; ok {
info.Description = desc
for _, r := range routes {
info.Routes = append(info.Routes, clientTypes.Route{
Path: r.Path,
Destination: r.Destination,
Content: r.Content,
HTTPSOnly: r.HTTPSOnly,
})
}

info.Tags = strings.Split(instance.ObjectMeta.Annotations["tags"], ",")

setInfoTeam(instance, info)

for _, ingress := range ingresses {
info.Addresses = append(info.Addresses, clientTypes.InstanceAddress{
Hostname: ingress.Hostname,
Expand All @@ -1420,6 +1421,10 @@ func newInstanceInfo(instance *v1alpha1.RpaasInstance, ingresses []corev1.LoadBa
func (m *k8sRpaasManager) getLoadBalancerIngress(ctx context.Context, instance *v1alpha1.RpaasInstance) ([]corev1.LoadBalancerIngress, error) {
var nginx nginxv1alpha1.Nginx
err := m.cli.Get(ctx, types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}, &nginx)
if err != nil && k8sErrors.IsNotFound(err) {
return nil, nil
}

if err != nil {
return nil, err
}
Expand All @@ -1438,37 +1443,21 @@ func (m *k8sRpaasManager) getLoadBalancerIngress(ctx context.Context, instance *
return svc.Status.LoadBalancer.Ingress, nil
}

func setInfoTeam(instance *v1alpha1.RpaasInstance, infoPayload *clientTypes.InstanceInfo) {
teamLabelKey := labelKey("team-owner")
team, ok := instance.ObjectMeta.Annotations[teamLabelKey]
if ok {
infoPayload.Team = team
return
}

team, ok = instance.Labels[teamLabelKey]
if ok {
infoPayload.Team = team
return
}

team, ok = instance.Spec.PodTemplate.Labels[teamLabelKey]
if ok {
infoPayload.Team = team
return
}
}

func (m *k8sRpaasManager) GetInstanceInfo(ctx context.Context, instanceName string) (*clientTypes.InstanceInfo, error) {
instance, err := m.GetInstance(ctx, instanceName)
if err != nil {
return nil, err
}

routes, err := m.GetRoutes(ctx, instanceName)
if err != nil {
return nil, err
}

ingresses, err := m.getLoadBalancerIngress(ctx, instance)
if err != nil {
return nil, err
}

return newInstanceInfo(instance, ingresses), nil
return newInstanceInfo(instance, routes, ingresses), nil
}
157 changes: 157 additions & 0 deletions internal/pkg/rpaas/k8s_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3571,3 +3571,160 @@ func Test_k8sRpaasManager_DeleteAutoscale(t *testing.T) {
})
}
}

func Test_k8sRpaasManager_GetInstanceInfo(t *testing.T) {
instance1 := newEmptyRpaasInstance()
instance1.Name = "instance1"
instance1.Annotations = map[string]string{
"rpaas.extensions.tsuru.io/description": "Some description about this instance",
"rpaas.extensions.tsuru.io/tags": "tag1,tag2,tag3",
"rpaas.extensions.tsuru.io/team-owner": "tsuru",
}
instance1.Spec.PlanName = "huge"

instance2 := instance1.DeepCopy()
instance2.Name = "instance2"
instance2.Spec.Replicas = pointerToInt(3)
instance2.Spec.Autoscale = &v1alpha1.RpaasInstanceAutoscaleSpec{
MaxReplicas: 100,
MinReplicas: pointerToInt(1),
TargetCPUUtilizationPercentage: pointerToInt(90),
}

instance3 := instance1.DeepCopy()
instance3.Name = "instance3"
instance3.Spec.Locations = []v1alpha1.Location{
{
Path: "/custom/path/1",
Destination: "app1.tsuru.example.com",
},
{
Path: "/custom/path/2",
Content: &v1alpha1.Value{
Value: "# some nginx configuration",
},
},
{
Path: "/custom/path/3",
Destination: "app3.tsuru.example.com",
ForceHTTPS: true,
},
}

instance4 := instance1.DeepCopy()
instance4.Name = "instance4"

service4 := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: instance4.Name + "-service",
Namespace: instance4.Namespace,
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
IP: "192.168.10.10",
Hostname: "instance4-service.rpaasv2.tsuru.example.com",
},
},
},
},
}

nginx4 := &nginxv1alpha1.Nginx{
ObjectMeta: metav1.ObjectMeta{
Name: instance4.Name,
Namespace: instance4.Namespace,
},
Status: nginxv1alpha1.NginxStatus{
Services: []nginxv1alpha1.ServiceStatus{
{Name: service4.Name},
},
},
}

resources := []runtime.Object{instance1, instance2, instance3, instance4, nginx4, service4}

testCases := []struct {
instance string
expected clientTypes.InstanceInfo
}{
{
instance: "instance1",
expected: clientTypes.InstanceInfo{
Name: "instance1",
Description: "Some description about this instance",
Team: "tsuru",
Tags: []string{"tag1", "tag2", "tag3"},
Plan: "huge",
},
},
{
instance: "instance2",
expected: clientTypes.InstanceInfo{
Name: "instance2",
Description: "Some description about this instance",
Team: "tsuru",
Tags: []string{"tag1", "tag2", "tag3"},
Plan: "huge",
Replicas: pointerToInt(3),
Autoscale: &clientTypes.Autoscale{
MaxReplicas: pointerToInt(100),
MinReplicas: pointerToInt(1),
CPU: pointerToInt(90),
},
},
},
{
instance: "instance3",
expected: clientTypes.InstanceInfo{
Name: "instance3",
Description: "Some description about this instance",
Team: "tsuru",
Tags: []string{"tag1", "tag2", "tag3"},
Plan: "huge",
Routes: []clientTypes.Route{
{
Path: "/custom/path/1",
Destination: "app1.tsuru.example.com",
},
{
Path: "/custom/path/2",
Content: "# some nginx configuration",
},
{
Path: "/custom/path/3",
Destination: "app3.tsuru.example.com",
HTTPSOnly: true,
},
},
},
},
{
instance: "instance4",
expected: clientTypes.InstanceInfo{
Name: "instance4",
Description: "Some description about this instance",
Team: "tsuru",
Tags: []string{"tag1", "tag2", "tag3"},
Plan: "huge",
Addresses: []clientTypes.InstanceAddress{
{
IP: "192.168.10.10",
Hostname: "instance4-service.rpaasv2.tsuru.example.com",
},
},
},
},
}

for _, tt := range testCases {
t.Run("", func(t *testing.T) {
manager := &k8sRpaasManager{cli: fake.NewFakeClientWithScheme(newScheme(), resources...)}
got, err := manager.GetInstanceInfo(context.Background(), tt.instance)
require.NoError(t, err)
require.NotNil(t, got)
assert.Equal(t, tt.expected, *got)
})
}
}

0 comments on commit 17c911b

Please sign in to comment.