Change default credentials manager check and not-found dialog

[phaumer]

Proposed changes

On systems on which the default credentials manager is not available, such as Eclipse Che, activation would run into an endless loop of prompting the user to accept disabling secure credentials, reload, and the same dialog again. This was caused by code that when reading the setting.json file would rewrite it again and triggering the new file watcher to reload it again. Asynchronous behavior would also intermingle prompts to the user of broken team configuration files as it tried to load credentials using the non-existent credentials manager.

I rewrote the code to add an independent check for the default credentials manager. It tests if it can be loaded by trying ProfilesCache.requireKeyring() and only calling the setupDefaultCredentialManager() when it succeeds. Before, it was always calling it wasting cycles by catching more exceptions.

I also changed the prompt to an info dialog (could discuss if it should be warning) with the Reload button only, to tell the user that credential management was disabled and they should consider installing a credentials manager (such as the K8s one). The reason is that the user really does not have a choice and prompting makes no difference. If they would select the No in the old dialog, it would just prompt them again after reload. Also storing the setting only makes sense for the global scope as the credentials manager would not be available for other workspaces either. So that button in the dialog is also not needed. Therefore, the only button left now is the Reload button. When clicked, the setting is stored, the settings.json is written, and the editor reloads without any additional prompts.

Release Notes

Milestone:

Changelog:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Enhancement (non-breaking change which adds or improves functionality)
• Breaking change (a change that would cause existing functionality to not work as expected)
• Documentation (Markdown, README updates)
• Other (please specify above in "Proposed changes" section)

Checklist

General

• I have read the CONTRIBUTOR GUIDANCE wiki
• All PR dependencies have been merged and published (if applicable)
• A GIF or screenshot is included in the PR for visual changes
• The pre-publish command has been executed:
  • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
  • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

• There is coverage for the code that I have added
• I have added new test cases and they are passing
• I have manually tested the changes

Deployment

• I have added developer documentation (if applicable)
• Documentation should be added to Zowe Docs
  • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
  • Otherwise, please check with the rest of the squad about any needed documentation before merging.
• These changes may need ported to the appropriate branches (list here):

Further comments

[codecov]

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.97%. Comparing base (e07e839) to head (071ec81).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/zowe-explorer/src/utils/ProfilesUtils.ts	93.75%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3297      +/-   ##
==========================================
- Coverage   92.99%   92.97%   -0.03%     
==========================================
  Files         116      116              
  Lines       12008    12020      +12     
  Branches     2753     2742      -11     
==========================================
+ Hits        11167    11175       +8     
- Misses        839      843       +4     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

📅 Suggested merge-by date: 11/21/2024

[JillieBeanSim]

LGTM! Thanks @phaumer for the fix, works great in the RH CRW environment disabling the secure creds and after refresh was prompted to create a new config and it created with the autoStore: false 🎉

[rudyflores]

Code and functionality looks great, tested this on a browser based environment and seems to fix this issue. Thanks @phaumer !

[zFernand0]

code changes make sense to me 🥳

left a couple of comments to hopefully avoid some duplication (if possible) 😋

It seems that most/all comments will be addressed in future PRs 😋

[zFernand0]

Apologies for the nitpicky comments.
Changes LGTM! 😋

Thanks for the fix!

[traeok]

Could we add the error message to the logger here in case we run into an issue with loading the credential manager? Otherwise, it could be difficult to track down why the default credential manager failed to load (could be useful when diagnosing credential manager bugs / customer issues).

[phaumer]

I don't want to log an error here as this will be the normal case for a container-based environment. I added more details about why this info message could have happened. What about that?

[traeok]

Logging as info makes sense, thanks for adding that 👍

[zFernand0]

Curious if @anaxceron got a chance to review these 😋
LGTM! 😋