Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): return 404 when the app is not found if a project is specified (#13393) #13394

Merged
merged 4 commits into from
Jul 19, 2023

Conversation

crenshaw-dev
Copy link
Member

Fixes #13393

@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Patch coverage: 60.25% and project coverage change: -0.02 ⚠️

Comparison is base (1f88d02) 49.83% compared to head (b00ad50) 49.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13394      +/-   ##
==========================================
- Coverage   49.83%   49.81%   -0.02%     
==========================================
  Files         261      261              
  Lines       44784    44826      +42     
==========================================
+ Hits        22316    22329      +13     
- Misses      20271    20296      +25     
- Partials     2197     2201       +4     
Impacted Files Coverage Δ
server/application/application.go 50.86% <60.25%> (-0.45%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

server/application/application.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@leoluz leoluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my comment

server/application/application.proto Outdated Show resolved Hide resolved
Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crenshaw-dev There seem to be some conflicts.. also some minor nits on the comments

server/application/application.go Show resolved Hide resolved
server/application/application.go Show resolved Hide resolved
…ied (argoproj#13393)

Signed-off-by: Michael Crenshaw <[email protected]>

simplify, respond 404 on project specified but doesn't match, always fetch app

Signed-off-by: Michael Crenshaw <[email protected]>

handle project updates

Signed-off-by: Michael Crenshaw <[email protected]>
Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crenshaw-dev crenshaw-dev merged commit 208852c into argoproj:master Jul 19, 2023
@crenshaw-dev crenshaw-dev deleted the 404-app-not-found branch July 19, 2023 13:22
@crenshaw-dev
Copy link
Member Author

/cherry-pick release-2.8

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Jul 19, 2023
…ied (#13393) (#13394)

* fix(api): return 404 when the app is not found if a project is specified (#13393)

Signed-off-by: Michael Crenshaw <[email protected]>

simplify, respond 404 on project specified but doesn't match, always fetch app

Signed-off-by: Michael Crenshaw <[email protected]>

handle project updates

Signed-off-by: Michael Crenshaw <[email protected]>

* handle new endpoint, fix bad merge

Signed-off-by: Michael Crenshaw <[email protected]>

* docs

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
crenshaw-dev added a commit that referenced this pull request Jul 19, 2023
…ied (#13393) (#13394) (#14600)

* fix(api): return 404 when the app is not found if a project is specified (#13393)



simplify, respond 404 on project specified but doesn't match, always fetch app



handle project updates



* handle new endpoint, fix bad merge



* docs



---------

Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this pull request Aug 9, 2023
…ied (argoproj#13393) (argoproj#13394)

* fix(api): return 404 when the app is not found if a project is specified (argoproj#13393)

Signed-off-by: Michael Crenshaw <[email protected]>

simplify, respond 404 on project specified but doesn't match, always fetch app

Signed-off-by: Michael Crenshaw <[email protected]>

handle project updates

Signed-off-by: Michael Crenshaw <[email protected]>

* handle new endpoint, fix bad merge

Signed-off-by: Michael Crenshaw <[email protected]>

* docs

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
@suzaku suzaku mentioned this pull request Aug 15, 2023
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
…ied (argoproj#13393) (argoproj#13394)

* fix(api): return 404 when the app is not found if a project is specified (argoproj#13393)

Signed-off-by: Michael Crenshaw <[email protected]>

simplify, respond 404 on project specified but doesn't match, always fetch app

Signed-off-by: Michael Crenshaw <[email protected]>

handle project updates

Signed-off-by: Michael Crenshaw <[email protected]>

* handle new endpoint, fix bad merge

Signed-off-by: Michael Crenshaw <[email protected]>

* docs

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Aggressively check RBAC when Application request is fully-parameterized
4 participants