Skip to content

Commit a16f85b

Browse files
aali309aali309
committed
validate managed-by-url
Signed-off-by: Atif Ali <[email protected]>
1 parent 157eea3 commit a16f85b

File tree

13 files changed

+236
-25
lines changed

13 files changed

+236
-25
lines changed

‎common/common.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,6 @@ const (
230230
// Skip reconcile when the value is "true" or any other string values that can be strconv.ParseBool() to be true.
231231
AnnotationKeyAppSkipReconcile = "argocd.argoproj.io/skip-reconcile"
232232

233-
// AnnotationKeyManagedByURL contains the URL of the Argo CD instance managing the application
234-
AnnotationKeyManagedByURL = "argocd.argoproj.io/managed-by-url"
235233
// LabelKeyComponentRepoServer is the label key to identify the component as repo-server
236234
LabelKeyComponentRepoServer = "app.kubernetes.io/component"
237235
// LabelValueComponentRepoServer is the label value for the repo-server component

‎controller/cache/info.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ func populateHostNodeInfo(un *unstructured.Unstructured, res *ResourceInfo) {
494494

495495
func populateApplicationInfo(un *unstructured.Unstructured, res *ResourceInfo) {
496496
// Add managed-by-url annotation to info if present
497-
if managedByURL, ok := un.GetAnnotations()[common.AnnotationKeyManagedByURL]; ok {
497+
if managedByURL, ok := un.GetAnnotations()[v1alpha1.AnnotationKeyManagedByURL]; ok {
498498
res.Info = append(res.Info, v1alpha1.InfoItem{Name: "managed-by-url", Value: managedByURL})
499499
}
500500
}

‎pkg/apis/application/v1alpha1/application_annotations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,6 @@ const (
1111
// absolute path means an absolute path within the repository and the relative path is relative to the application
1212
// source path within the repository.
1313
AnnotationKeyManifestGeneratePaths = "argocd.argoproj.io/manifest-generate-paths"
14+
// AnnotationKeyManagedByURL contains the URL of the Argo CD instance managing the application
15+
AnnotationKeyManagedByURL = "argocd.argoproj.io/managed-by-url"
1416
)

‎server/application/application.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,12 @@ func (s *Server) validateAndNormalizeApp(ctx context.Context, app *v1alpha1.Appl
13571357
return status.Errorf(codes.InvalidArgument, "application spec for %s is invalid: %s", app.Name, argo.FormatAppConditions(conditions))
13581358
}
13591359

1360+
// Validate managed-by-url annotation
1361+
managedByURLConditions := argo.ValidateManagedByURL(app)
1362+
if len(managedByURLConditions) > 0 {
1363+
return status.Errorf(codes.InvalidArgument, "application spec for %s is invalid: %s", app.Name, argo.FormatAppConditions(managedByURLConditions))
1364+
}
1365+
13601366
app.Spec = *argo.NormalizeApplicationSpec(&app.Spec)
13611367
return nil
13621368
}

‎server/cluster/cluster_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ func TestCreateDeepLinksObject_ManagedByURL(t *testing.T) {
864864
Object: map[string]any{
865865
"metadata": map[string]any{
866866
"annotations": map[string]any{
867-
common.AnnotationKeyManagedByURL: "https://example.com/argo",
867+
appv1.AnnotationKeyManagedByURL: "https://example.com/argo",
868868
},
869869
},
870870
},

‎server/deeplinks/deeplinks.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1212
"k8s.io/utils/ptr"
1313

14-
"github.com/argoproj/argo-cd/v3/common"
1514
"github.com/argoproj/argo-cd/v3/pkg/apiclient/application"
1615
"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
1716
"github.com/argoproj/argo-cd/v3/util/settings"
@@ -77,7 +76,7 @@ func CreateDeepLinksObject(resourceObj *unstructured.Unstructured, app *unstruct
7776
if app.Object["metadata"] != nil {
7877
if metadata, ok := app.Object["metadata"].(map[string]any); ok {
7978
if annotations, ok := metadata["annotations"].(map[string]any); ok {
80-
if managedByURL, ok := annotations[common.AnnotationKeyManagedByURL].(string); ok {
79+
if managedByURL, ok := annotations[v1alpha1.AnnotationKeyManagedByURL].(string); ok {
8180
deeplinkObj[ManagedByURLKey] = managedByURL
8281
}
8382
}

‎server/deeplinks/deeplinks_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"k8s.io/apimachinery/pkg/runtime"
1515
"k8s.io/utils/ptr"
1616

17-
"github.com/argoproj/argo-cd/v3/common"
1817
"github.com/argoproj/argo-cd/v3/pkg/apiclient/application"
1918
"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
2019
"github.com/argoproj/argo-cd/v3/util/settings"
@@ -222,7 +221,7 @@ func TestManagedByURLAnnotation(t *testing.T) {
222221
ObjectMeta: metav1.ObjectMeta{
223222
Name: "test-app",
224223
Annotations: map[string]string{
225-
common.AnnotationKeyManagedByURL: managedByURL,
224+
v1alpha1.AnnotationKeyManagedByURL: managedByURL,
226225
},
227226
},
228227
}
@@ -265,7 +264,7 @@ func TestManagedByURLAnnotation(t *testing.T) {
265264
ObjectMeta: metav1.ObjectMeta{
266265
Name: "test-app",
267266
Annotations: map[string]string{
268-
common.AnnotationKeyManagedByURL: "",
267+
v1alpha1.AnnotationKeyManagedByURL: "",
269268
},
270269
},
271270
}
@@ -290,9 +289,9 @@ func TestManagedByURLAnnotation(t *testing.T) {
290289
ObjectMeta: metav1.ObjectMeta{
291290
Name: "test-app",
292291
Annotations: map[string]string{
293-
common.AnnotationKeyManagedByURL: managedByURL,
294-
"argocd.argoproj.io/deep-link-1": "https://grafana.example.com/d/argo/argo-cd-application-dashboard",
295-
"argocd.argoproj.io/deep-link-2": "https://kibana.example.com/app/kibana#/discover",
292+
v1alpha1.AnnotationKeyManagedByURL: managedByURL,
293+
"argocd.argoproj.io/deep-link-1": "https://grafana.example.com/d/argo/argo-cd-application-dashboard",
294+
"argocd.argoproj.io/deep-link-2": "https://kibana.example.com/app/kibana#/discover",
296295
},
297296
},
298297
}

‎test/e2e/managed_by_url_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import (
99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010
"k8s.io/utils/ptr"
1111

12-
"github.com/argoproj/argo-cd/v3/common"
1312
"github.com/argoproj/argo-cd/v3/pkg/apiclient/application"
1413
"github.com/argoproj/argo-cd/v3/pkg/apiclient/settings"
14+
"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
1515
. "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
1616
"github.com/argoproj/argo-cd/v3/test/e2e/fixture"
1717
. "github.com/argoproj/argo-cd/v3/test/e2e/fixture/app"
@@ -38,7 +38,7 @@ func TestManagedByURLWithAnnotation(t *testing.T) {
3838
if appObj.Annotations == nil {
3939
appObj.Annotations = make(map[string]string)
4040
}
41-
appObj.Annotations[common.AnnotationKeyManagedByURL] = managedByURL
41+
appObj.Annotations[v1alpha1.AnnotationKeyManagedByURL] = managedByURL
4242

4343
_, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(fixture.ArgoCDNamespace).Update(t.Context(), appObj, metav1.UpdateOptions{})
4444
if err == nil {
@@ -75,7 +75,7 @@ func TestManagedByURLWithAnnotation(t *testing.T) {
7575
Then().
7676
And(func(app *Application) {
7777
// Test that the managed-by-url annotation is preserved
78-
assert.Equal(t, managedByURL, app.Annotations[common.AnnotationKeyManagedByURL])
78+
assert.Equal(t, managedByURL, app.Annotations[v1alpha1.AnnotationKeyManagedByURL])
7979

8080
// Test that the application links include the managed-by-url in the deep links
8181
conn, appClient, err := fixture.ArgoCDClientset.NewApplicationClient()

‎ui/src/app/applications/components/utils.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as moment from 'moment';
88
import {BehaviorSubject, combineLatest, concat, from, fromEvent, Observable, Observer, Subscription} from 'rxjs';
99
import {debounceTime, map} from 'rxjs/operators';
1010
import {AppContext, Context, ContextApis} from '../../shared/context';
11+
import {isValidURL} from '../../shared/utils';
1112
import {ResourceTreeNode} from './application-resource-tree/application-resource-tree';
1213

1314
import {CheckboxField, COLORS, ErrorNotification, Revision} from '../../shared/components';
@@ -1790,8 +1791,16 @@ export function getApplicationLinkURL(app: any, baseHref: string, node?: any): {
17901791

17911792
let url, isExternal;
17921793
if (managedByURL) {
1793-
url = managedByURL + '/applications/' + app.metadata.namespace + '/' + app.metadata.name;
1794-
isExternal = true;
1794+
// Validate the managed-by URL using the same validation as external links
1795+
if (!isValidURL(managedByURL)) {
1796+
// If URL is invalid, fall back to local URL for security
1797+
console.warn(`Invalid managed-by URL for application ${app.metadata.name}: ${managedByURL}`);
1798+
url = baseHref + 'applications/' + app.metadata.namespace + '/' + app.metadata.name;
1799+
isExternal = false;
1800+
} else {
1801+
url = managedByURL + '/applications/' + app.metadata.namespace + '/' + app.metadata.name;
1802+
isExternal = true;
1803+
}
17951804
} else {
17961805
url = baseHref + 'applications/' + app.metadata.namespace + '/' + app.metadata.name;
17971806
isExternal = false;
@@ -1810,8 +1819,16 @@ export function getApplicationLinkURLFromNode(node: any, baseHref: string): {url
18101819

18111820
let url, isExternal;
18121821
if (managedByURL) {
1813-
url = managedByURL + '/applications/' + node.namespace + '/' + node.name;
1814-
isExternal = true;
1822+
// Validate the managed-by URL using the same validation as external links
1823+
if (!isValidURL(managedByURL)) {
1824+
// If URL is invalid, fall back to local URL for security
1825+
console.warn(`Invalid managed-by URL for application ${node.name}: ${managedByURL}`);
1826+
url = baseHref + 'applications/' + node.namespace + '/' + node.name;
1827+
isExternal = false;
1828+
} else {
1829+
url = managedByURL + '/applications/' + node.namespace + '/' + node.name;
1830+
isExternal = true;
1831+
}
18151832
} else {
18161833
url = baseHref + 'applications/' + node.namespace + '/' + node.name;
18171834
isExternal = false;

‎util/argo/argo.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,29 @@ func ValidateRepo(
393393
return conditions, nil
394394
}
395395

396+
// ValidateManagedByURL validates the managed-by-url annotation on applications to ensure it contains a valid URL
397+
func ValidateManagedByURL(app *argoappv1.Application) []argoappv1.ApplicationCondition {
398+
conditions := make([]argoappv1.ApplicationCondition, 0)
399+
400+
if app.Annotations == nil {
401+
return conditions
402+
}
403+
404+
managedByURL, exists := app.Annotations[argoappv1.AnnotationKeyManagedByURL]
405+
if !exists || managedByURL == "" {
406+
return conditions
407+
}
408+
409+
if err := settings.ValidateExternalURL(managedByURL); err != nil {
410+
conditions = append(conditions, argoappv1.ApplicationCondition{
411+
Type: argoappv1.ApplicationConditionInvalidSpecError,
412+
Message: fmt.Sprintf("invalid managed-by URL: %v", err),
413+
})
414+
}
415+
416+
return conditions
417+
}
418+
396419
func validateRepo(ctx context.Context,
397420
app *argoappv1.Application,
398421
db db.ArgoDB,

0 commit comments

Comments
 (0)