refactor vault Plugin to remove AppRole enabled check and clarify separated classes #1835
Conversation
|
🤖 Hey ! The @cpn-console/vault-plugin (v2.3.0) package already exists on npm but the source code has changed, you should consider updating the package version. The version update warning should be ignored in the case of modifications that do not affect the code once it has been built, such as code formatting, etc... |
|
🤖 Hey ! The security scan report for the current pull request is available here. |
mathieulaude
changed the title
|
🤖 Hey ! The @cpn-console/harbor-plugin (v2.2.3) package already exists on npm but the source code has changed, you should consider updating the package version. The version update warning should be ignored in the case of modifications that do not affect the code once it has been built, such as code formatting, etc... |
|
🤖 Hey ! The @cpn-console/gitlab-plugin (v3.3.1) package already exists on npm but the source code has changed, you should consider updating the package version. The version update warning should be ignored in the case of modifications that do not affect the code once it has been built, such as code formatting, etc... |
|
🤖 Hey ! The @cpn-console/sonarqube-plugin (v2.0.5) package already exists on npm but the source code has changed, you should consider updating the package version. The version update warning should be ignored in the case of modifications that do not affect the code once it has been built, such as code formatting, etc... |
|
🤖 Hey ! The @cpn-console/argocd-plugin (v2.3.0) package already exists on npm but the source code has changed, you should consider updating the package version. The version update warning should be ignored in the case of modifications that do not affect the code once it has been built, such as code formatting, etc... |
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
No data about coverage (71.90% Estimated after merge)
| import monitor from './monitor.js' | ||
| import { VaultProjectApi, VaultZoneApi } from './class.js' | ||
| import { VaultProjectApi } from './vault-project-api.js' | ||
| import { VaultZoneApi } from './vault-zone-api.js' |
There was a problem hiding this comment.
praise: C'est beau !
| private readonly roleName: string | ||
| private readonly projectRootDir: string | ||
| private readonly defaultAppRoleCredentials: AppRoleCredentials | ||
| private readonly coreKvName: string = 'forge-dso' |
There was a problem hiding this comment.
thought: Rien à voir avec cette MR, mais cette valeur en dur me dérange à plus d'un titre
| this.defaultAppRoleCredentials = { | ||
| url: getConfig().deployVaultConnectionInNs ? getConfig().publicUrl : '', | ||
| coreKvName: this.coreKvName, | ||
| roleId: 'none', |
There was a problem hiding this comment.
question: Je sais que c'était comme ça auparavant, mais est-ce que ce n'est pas un problème ? 'none' ce n'est pas un roleId, en soi. undefined (et un typage adapté) eut probablement été plus clair, non ?
|
🤖 Hey ! A preview of the application is available at : https://console-pr-1835.dso.cpin-hp.numerique-interieur.fr Please be patient, deployment may take a few minutes. |
StephaneTrebel
added
preview
C'est ok ! 🚀 |
Issues liées
Issues numéro: #1829
Quel est le comportement actuel ?
Quel est le nouveau comportement ?
Cette PR introduit-elle un breaking change ?
Autres informations