Skip to content

Commit 9708505

Browse files
committed
fix(webhook): render placeholders when a webhook policy has no targets
harbor webhook list panicked with an index-out-of-range error whenever Harbor returned a webhook policy with an empty targets array. The list view indexed Targets[0] unconditionally, but targets is optional in the generated WebhookPolicy model. Extract row construction into webhookRows and fall back to a placeholder for the endpoint URL, notify type and payload format when the target is missing, nil or empty. Nil policies are skipped rather than dereferenced. Fixes #973 Signed-off-by: Somil Gupta <gsomil93@gmail.com>
1 parent d98afec commit 9708505

2 files changed

Lines changed: 143 additions & 13 deletions

File tree

pkg/views/webhook/list/view.go

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,34 +35,61 @@ var columns = []table.Column{
3535
{Title: "Creation Time", Width: 20},
3636
}
3737

38+
// placeholder is shown whenever a webhook policy does not carry the target
39+
// details Harbor optionally omits from the response.
40+
const placeholder = "--"
41+
3842
func ListWebhooks(webhooks []*models.WebhookPolicy) {
43+
rows := webhookRows(webhooks)
44+
m := tablelist.NewModel(columns, rows, len(rows))
45+
46+
if _, err := tea.NewProgram(m).Run(); err != nil {
47+
fmt.Println("Error running program:", err)
48+
os.Exit(1)
49+
}
50+
}
51+
52+
// webhookRows renders the table rows for the given policies. Targets are
53+
// optional in the Harbor API model, so a policy without any target still
54+
// yields a row with placeholders instead of panicking.
55+
func webhookRows(webhooks []*models.WebhookPolicy) []table.Row {
3956
var rows []table.Row
4057
for _, webhook := range webhooks {
41-
var webhookEnabled string
58+
if webhook == nil {
59+
continue
60+
}
61+
62+
webhookEnabled := "False"
4263
if webhook.Enabled {
4364
webhookEnabled = "True"
44-
} else {
45-
webhookEnabled = "False"
4665
}
47-
payloadFormat := "--"
48-
if len(webhook.Targets[0].PayloadFormat) != 0 {
49-
payloadFormat = string(webhook.Targets[0].PayloadFormat)
66+
67+
endpointURL, notifyType := placeholder, placeholder
68+
payloadFormat := placeholder
69+
if len(webhook.Targets) > 0 && webhook.Targets[0] != nil {
70+
target := webhook.Targets[0]
71+
if target.Address != "" {
72+
endpointURL = target.Address
73+
}
74+
if target.Type != "" {
75+
notifyType = target.Type
76+
}
77+
if target.PayloadFormat != "" {
78+
payloadFormat = string(target.PayloadFormat)
79+
}
5080
}
81+
5182
creationTime, _ := utils.FormatCreatedTime(webhook.CreationTime.String())
5283
rows = append(rows, table.Row{
5384
strconv.FormatInt(webhook.ID, 10),
5485
webhook.Name,
5586
webhookEnabled,
56-
webhook.Targets[0].Address,
57-
webhook.Targets[0].Type,
87+
endpointURL,
88+
notifyType,
5889
payloadFormat,
5990
creationTime,
6091
})
6192
}
62-
m := tablelist.NewModel(columns, rows, len(rows))
6393

64-
if _, err := tea.NewProgram(m).Run(); err != nil {
65-
fmt.Println("Error running program:", err)
66-
os.Exit(1)
67-
}
94+
return rows
6895
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright Project Harbor Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package list
15+
16+
import (
17+
"testing"
18+
19+
"github.com/goharbor/go-client/pkg/sdk/v2.0/models"
20+
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
22+
)
23+
24+
func TestWebhookRows(t *testing.T) {
25+
t.Run("policy with a target", func(t *testing.T) {
26+
rows := webhookRows([]*models.WebhookPolicy{
27+
{
28+
ID: 1,
29+
Name: "hook",
30+
Enabled: true,
31+
Targets: []*models.WebhookTargetObject{
32+
{
33+
Address: "https://example.com/hook",
34+
Type: "http",
35+
PayloadFormat: models.PayloadFormatType("CloudEvents"),
36+
},
37+
},
38+
},
39+
})
40+
41+
require.Len(t, rows, 1)
42+
assert.Equal(t, "1", rows[0][0])
43+
assert.Equal(t, "hook", rows[0][1])
44+
assert.Equal(t, "True", rows[0][2])
45+
assert.Equal(t, "https://example.com/hook", rows[0][3])
46+
assert.Equal(t, "http", rows[0][4])
47+
assert.Equal(t, "CloudEvents", rows[0][5])
48+
})
49+
50+
t.Run("policy without targets falls back to placeholders", func(t *testing.T) {
51+
rows := webhookRows([]*models.WebhookPolicy{
52+
{ID: 2, Name: "no-targets"},
53+
})
54+
55+
require.Len(t, rows, 1)
56+
assert.Equal(t, "2", rows[0][0])
57+
assert.Equal(t, "no-targets", rows[0][1])
58+
assert.Equal(t, "False", rows[0][2])
59+
assert.Equal(t, placeholder, rows[0][3])
60+
assert.Equal(t, placeholder, rows[0][4])
61+
assert.Equal(t, placeholder, rows[0][5])
62+
})
63+
64+
t.Run("policy with a nil target falls back to placeholders", func(t *testing.T) {
65+
rows := webhookRows([]*models.WebhookPolicy{
66+
{ID: 3, Name: "nil-target", Targets: []*models.WebhookTargetObject{nil}},
67+
})
68+
69+
require.Len(t, rows, 1)
70+
assert.Equal(t, placeholder, rows[0][3])
71+
assert.Equal(t, placeholder, rows[0][4])
72+
assert.Equal(t, placeholder, rows[0][5])
73+
})
74+
75+
t.Run("target with empty fields falls back to placeholders", func(t *testing.T) {
76+
rows := webhookRows([]*models.WebhookPolicy{
77+
{ID: 4, Name: "empty-target", Targets: []*models.WebhookTargetObject{{}}},
78+
})
79+
80+
require.Len(t, rows, 1)
81+
assert.Equal(t, placeholder, rows[0][3])
82+
assert.Equal(t, placeholder, rows[0][4])
83+
assert.Equal(t, placeholder, rows[0][5])
84+
})
85+
86+
t.Run("nil policies are skipped", func(t *testing.T) {
87+
rows := webhookRows([]*models.WebhookPolicy{nil, {ID: 5, Name: "kept"}})
88+
89+
require.Len(t, rows, 1)
90+
assert.Equal(t, "kept", rows[0][1])
91+
})
92+
93+
t.Run("row has one cell per column", func(t *testing.T) {
94+
rows := webhookRows([]*models.WebhookPolicy{{ID: 6}})
95+
96+
require.Len(t, rows, 1)
97+
assert.Len(t, rows[0], len(columns))
98+
})
99+
100+
t.Run("no policies yields no rows", func(t *testing.T) {
101+
assert.Empty(t, webhookRows(nil))
102+
})
103+
}

0 commit comments

Comments
 (0)