Skip to content

Commit

Permalink
fix: invalid badge validation (argoproj#15507) (argoproj#17580)
Browse files Browse the repository at this point in the history 
* fix: invalid badge validation

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* use util methods

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* rfc accept both lower and upper

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* fix unit test affecting each other with var modification

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

---------

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Co-authored-by: Jann Fischer <jann@mistrust.net>
Signed-off-by: Kevin Lyda <kevin@lyda.ie>
  • Loading branch information
@agaudreault @jannfis @lyda
2 people authored and lyda committed Mar 28, 2024
1 parent 5a7a2cd commit 8d75dad
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 35 deletions.
4 changes: 2 additions & 2 deletions server/badge/badge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

reqNs := ""
if ns, ok := r.URL.Query()["namespace"]; ok && enabled {
if errs := validation.NameIsDNSSubdomain(strings.ToLower(ns[0]), false); len(errs) == 0 {
if argo.IsValidNamespaceName(ns[0]) {
if security.IsNamespaceEnabled(ns[0], h.namespace, h.enabledNamespaces) {
reqNs = ns[0]
} else {
Expand All @@ -117,7 +117,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

//Sample url: http://localhost:8080/api/badge?name=123
if name, ok := r.URL.Query()["name"]; ok && enabled && !notFound {
if errs := validation.NameIsDNSLabel(strings.ToLower(name[0]), false); len(errs) == 0 {
if argo.IsValidAppName(name[0]) {
if app, err := h.appClientset.ArgoprojV1alpha1().Applications(reqNs).Get(context.Background(), name[0], v1.GetOptions{}); err == nil {
health = app.Status.Health.Status
status = app.Status.Sync.Status
Expand Down
92 changes: 59 additions & 33 deletions server/badge/badge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,22 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/fake"
)

var (
argoCDSecret = corev1.Secret{
func argoCDSecret() *corev1.Secret {
return &corev1.Secret{
ObjectMeta: v1.ObjectMeta{Name: "argocd-secret", Namespace: "default"},
Data: map[string][]byte{
"admin.password": []byte("test"),
"server.secretkey": []byte("test"),
},
}
argoCDCm = corev1.ConfigMap{
}

func argoCDCm() *corev1.ConfigMap {
return &corev1.ConfigMap{
ObjectMeta: v1.ObjectMeta{
Name: "argocd-cm",
Namespace: "default",
Expand All @@ -41,7 +45,10 @@ var (
"statusbadge.enabled": "true",
},
}
testApp = v1alpha1.Application{
}

func testApp() *v1alpha1.Application {
return &v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "default"},
Status: v1alpha1.ApplicationStatus{
Sync: v1alpha1.SyncStatus{Status: v1alpha1.SyncStatusCodeSynced},
Expand All @@ -53,7 +60,9 @@ var (
},
},
}
testApp2 = v1alpha1.Application{
}
func testApp2() *v1alpha1.Application {
return &v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "argocd-test"},
Status: v1alpha1.ApplicationStatus{
Sync: v1alpha1.SyncStatus{Status: v1alpha1.SyncStatusCodeSynced},
Expand All @@ -65,15 +74,17 @@ var (
},
},
}
testProject = v1alpha1.AppProject{
}
func testProject() *v1alpha1.AppProject {
return &v1alpha1.AppProject{
ObjectMeta: v1.ObjectMeta{Name: "test-project", Namespace: "default"},
Spec: v1alpha1.AppProjectSpec{},
}
)
}

func TestHandlerFeatureIsEnabled(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp), settingsMgr, "default", []string{})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp()), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app", nil)
assert.NoError(t, err)

Expand Down Expand Up @@ -129,12 +140,23 @@ func TestHandlerFeatureProjectIsEnabled(t *testing.T) {
http.StatusBadRequest, "/api/badge?name=foo_bar", "default", "Unknown", "Unknown", Purple, Purple},
{createApplications([]string{"Unknown:Unknown", "Unknown:Unknown"}, []string{"test-project", "default"}, "default"),
http.StatusOK, "/api/badge?name=foobar", "default", "Not Found", "", Purple, Purple},
{createApplicationsWithName([]string{"Healthy:Synced"}, []string{"default"}, "test", "test.application"),
http.StatusOK, "/api/badge?name=test.application-0", "test", "Healthy", "Synced", Green, Green},
{createApplicationsWithName([]string{"Healthy:Synced"}, []string{"default"}, "test", "test.invalid_name"),
http.StatusBadRequest, "/api/badge?name=test.invalid_name-0", "test", "Healthy", "Synced", Green, Green},
}
for _, tt := range projectTests {
argoCDCm := argoCDCm()
argoCDSecret := argoCDSecret()
argoCDCm.ObjectMeta.Namespace = tt.namespace
argoCDSecret.ObjectMeta.Namespace = tt.namespace
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), tt.namespace)
handler := NewHandler(appclientset.NewSimpleClientset(&testProject, tt.testApp[0], tt.testApp[1]), settingsMgr, tt.namespace, []string{})

settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm, argoCDSecret), tt.namespace)
objects := []runtime.Object{testProject()}
for _, v := range tt.testApp {
objects = append(objects, v)
}
handler := NewHandler(appclientset.NewSimpleClientset(objects...), settingsMgr, tt.namespace, []string{})
rr := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, tt.apiEndPoint, nil)
assert.NoError(t, err)
Expand All @@ -156,8 +178,8 @@ func TestHandlerFeatureProjectIsEnabled(t *testing.T) {

func TestHandlerNamespacesIsEnabled(t *testing.T) {
t.Run("Application in allowed namespace", func(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp2), settingsMgr, "default", []string{"argocd-test"})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp2()), settingsMgr, "default", []string{"argocd-test"})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&namespace=argocd-test", nil)
assert.NoError(t, err)

Expand All @@ -177,15 +199,15 @@ func TestHandlerNamespacesIsEnabled(t *testing.T) {
})

t.Run("Application in disallowed namespace", func(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp2), settingsMgr, "default", []string{"argocd-test"})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp2()), settingsMgr, "default", []string{"argocd-test"})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&namespace=kube-system", nil)
assert.NoError(t, err)

rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)

assert.Equal(t, http.StatusOK, rr.Result().StatusCode)
require.Equal(t, http.StatusOK, rr.Result().StatusCode)
response := rr.Body.String()
assert.Equal(t, toRGBString(Purple), leftRectColorPattern.FindStringSubmatch(response)[1])
assert.Equal(t, toRGBString(Purple), rightRectColorPattern.FindStringSubmatch(response)[1])
Expand All @@ -195,15 +217,15 @@ func TestHandlerNamespacesIsEnabled(t *testing.T) {
})

t.Run("Request with illegal namespace", func(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp2), settingsMgr, "default", []string{"argocd-test"})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp2()), settingsMgr, "default", []string{"argocd-test"})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&namespace=kube()system", nil)
assert.NoError(t, err)

rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)

assert.Equal(t, http.StatusBadRequest, rr.Result().StatusCode)
require.Equal(t, http.StatusBadRequest, rr.Result().StatusCode)
})
}

Expand All @@ -224,6 +246,10 @@ func createApplicationFeatureProjectIsEnabled(healthStatus health.HealthStatusCo
}

func createApplications(appCombo, projectName []string, namespace string) []*v1alpha1.Application {
return createApplicationsWithName(appCombo, projectName, namespace, "app")
}

func createApplicationsWithName(appCombo, projectName []string, namespace string, namePrefix string) []*v1alpha1.Application {
apps := make([]*v1alpha1.Application, len(appCombo))
healthStatus := func(healthType string) health.HealthStatusCode {
switch healthType {
Expand All @@ -249,14 +275,14 @@ func createApplications(appCombo, projectName []string, namespace string) []*v1a
a := strings.Split(v, ":")
healthApp := healthStatus(a[0])
syncApp := syncStatus(a[1])
appName := fmt.Sprintf("App %v", k)
appName := fmt.Sprintf("%s-%v", namePrefix, k)
apps[k] = createApplicationFeatureProjectIsEnabled(healthApp, syncApp, appName, projectName[k], namespace)
}
return apps
}
func TestHandlerFeatureIsEnabledRevisionIsEnabled(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp), settingsMgr, "default", []string{})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp()), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&revision=true", nil)
assert.NoError(t, err)

Expand All @@ -276,10 +302,10 @@ func TestHandlerFeatureIsEnabledRevisionIsEnabled(t *testing.T) {
}

func TestHandlerRevisionIsEnabledNoOperationState(t *testing.T) {
app := testApp.DeepCopy()
app := testApp()
app.Status.OperationState = nil

settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(app), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&revision=true", nil)
assert.NoError(t, err)
Expand All @@ -300,10 +326,10 @@ func TestHandlerRevisionIsEnabledNoOperationState(t *testing.T) {
}

func TestHandlerRevisionIsEnabledShortCommitSHA(t *testing.T) {
app := testApp.DeepCopy()
app := testApp()
app.Status.OperationState.SyncResult.Revision = "abc"

settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(app), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&revision=true", nil)
assert.NoError(t, err)
Expand All @@ -317,11 +343,11 @@ func TestHandlerRevisionIsEnabledShortCommitSHA(t *testing.T) {

func TestHandlerFeatureIsDisabled(t *testing.T) {

argoCDCmDisabled := argoCDCm.DeepCopy()
argoCDCmDisabled := argoCDCm()
delete(argoCDCmDisabled.Data, "statusbadge.enabled")

settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCmDisabled, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp), settingsMgr, "default", []string{})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCmDisabled, argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp()), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app", nil)
assert.NoError(t, err)

Expand All @@ -341,8 +367,8 @@ func TestHandlerFeatureIsDisabled(t *testing.T) {
}

func TestHandlerApplicationNameInBadgeIsEnabled(t *testing.T) {
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp), settingsMgr, "default", []string{})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp()), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app&showAppName=true", nil)
assert.NoError(t, err)

Expand All @@ -369,8 +395,8 @@ func TestHandlerApplicationNameInBadgeIsEnabled(t *testing.T) {

func TestHandlerApplicationNameInBadgeIsDisabled(t *testing.T) {

settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(&argoCDCm, &argoCDSecret), "default")
handler := NewHandler(appclientset.NewSimpleClientset(&testApp), settingsMgr, "default", []string{})
settingsMgr := settings.NewSettingsManager(context.Background(), fake.NewSimpleClientset(argoCDCm(), argoCDSecret()), "default")
handler := NewHandler(appclientset.NewSimpleClientset(testApp()), settingsMgr, "default", []string{})
req, err := http.NewRequest(http.MethodGet, "/api/badge?name=test-app", nil)
assert.NoError(t, err)

Expand Down

0 comments on commit 8d75dad

Please sign in to comment.