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

feat: Add new command to check local-source-proxy status #524

Merged
merged 4 commits into from
Apr 11, 2023

Conversation

odinnordico
Copy link
Contributor

@odinnordico odinnordico commented Apr 3, 2023

Pull request

What this PR does / why we need it

Although in TAP 1.6+, LSP will be provisioned by default on TAP iterate clusters, we can’t assume developers will always be pointing to a TAP env where LSP exists or is running/healthy.

This PR provides an apps command that can be invoked by the IDE extensions to determine whether LSP is installed/healthy

A new CLI command that checks:

  • Does the user have RBAC permission to talk to LSP?
  • Is LSP installed on the cluster?
  • Can LSP talk to the upstream repo that is configured by the operator?

Which issue(s) this PR fixes

Fixes #523

Describe testing done for PR

Testing against installation from the source code of Local Source Proxy, causing the scenarios described in the outputs

Check comment below

Additional information or special notes for your reviewer

@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2023

Codecov Report

Merging #524 (57af759) into main (4c55698) will decrease coverage by 0.31%.
The diff coverage is 76.71%.

@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
- Coverage   85.95%   85.65%   -0.31%     
==========================================
  Files          65       69       +4     
  Lines        4123     4265     +142     
==========================================
+ Hits         3544     3653     +109     
- Misses        453      477      +24     
- Partials      126      135       +9     
Impacted Files Coverage Δ
pkg/cli-runtime/client.go 66.20% <0.00%> (ø)
pkg/commands/localsourceproxy.go 0.00% <0.00%> (ø)
pkg/printer/local_source_proxy.go 64.70% <64.70%> (ø)
pkg/commands/lsp/lsp.go 80.48% <80.48%> (ø)
pkg/commands/localsourceproxy_health.go 100.00% <100.00%> (ø)
pkg/source/local_registry_client.go 54.16% <100.00%> (ø)
pkg/source/local_registry_round_tripper.go 64.28% <100.00%> (+1.78%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@odinnordico
Copy link
Contributor Author

  • Local Source Proxy healthy and well configured

image

  • Local Source Proxy miss configured:

image

  • Local Source Proxy well configured but auth error against registry

image

  • Local Source Proxy with no pods available

image

  • Local Source Proxy with internal errors

image

  • Local Source Proxy not installed

image

  • User's RBAC is not enough for requesting from Local Source Proxy
$ tanzu apps local-source-proxy health
user_has_permission: false
reachable: false
upstream_authenticated: false
overall_health: false
message: -|
  The current user does not have permission to access the local source proxy
  - services "http:local-source-proxy:5001" is forbidden: User "***@vmware.com" cannot get resource "services/proxy" in API group "" in the namespace "tap-local-source-system": requires one of ["container.services.proxy"] permission(s).

@odinnordico odinnordico marked this pull request as ready for review April 5, 2023 22:36
pkg/commands/lsp/lsp.go Show resolved Hide resolved
pkg/commands/lsp/lsp_test.go Outdated Show resolved Hide resolved
pkg/printer/local_source_proxy.go Outdated Show resolved Hide resolved
pkg/commands/localsourceproxy_health_test.go Outdated Show resolved Hide resolved
pkg/commands/localsourceproxy_health_test.go Outdated Show resolved Hide resolved
pkg/printer/local_source_proxy_test.go Outdated Show resolved Hide resolved
pkg/commands/localsourceproxy_health_test.go Outdated Show resolved Hide resolved
@warango4
Copy link
Contributor

Please rebase as there is a conflict with main branch

warango4 and others added 3 commits April 10, 2023 13:10
Signed-off-by: Wendy Arango <warango@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
Co-authored-by: Wendy Arango <warango@vmware.com>
Co-authored-by: Diego Alfonso<dalfonso@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
warango4
warango4 previously approved these changes Apr 10, 2023
pkg/commands/lsp/lsp.go Outdated Show resolved Hide resolved
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
Copy link
Contributor

@warango4 warango4 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
Contributor

@anibmurthy anibmurthy 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
Contributor

@shaheerkootteeri shaheerkootteeri left a comment

Choose a reason for hiding this comment

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

LGTM

@warango4 warango4 merged commit 9f3fce9 into vmware-tanzu:main Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tanzu apps lsp health for reporting health of the local source registry proxy
6 participants