feat: Azure Key Vault YAML setup support #285
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #285 +/- ##
==========================================
+ Coverage 93.89% 94.18% +0.29%
==========================================
Files 46 48 +2
Lines 1589 1634 +45
Branches 215 222 +7
==========================================
+ Hits 1492 1539 +47
+ Misses 97 95 -2
Continue to review full report at Codecov.
|
PIC123
force-pushed
the
phcherne/managed-identity-arm
branch
from
49443fc to
115e255
Compare
| expect(sls.cli.log).lastCalledWith("Finished KeyVault service setup") | ||
| }); | ||
|
|
||
| it("does not call deploy API or deploy functions when \"keyVault\" not included in config", async () => { |
There was a problem hiding this comment.
Is test name correct? I would imagine it not calling setPolicy or similar, right?
| let serverless: Serverless = MockFactory.createTestServerless(); | ||
|
|
||
| beforeEach(() => { | ||
| // KeyVaultManagementClient.prototype.vaults = { |
| expect(service).not.toBeNull(); | ||
| }); | ||
|
|
||
| it("Throws an error if correct keyvault name is not specified", async () => { |
There was a problem hiding this comment.
Test name seems off. The test shows that the vault doesn't exist - not that it was not specified.
| properties: knownVaults["testVault"].properties | ||
| } | ||
| ); | ||
| // fail(); |
src/services/azureKeyVaultService.ts
Outdated
| const identity = func.identity; | ||
|
|
||
| const keyVaultClient = new KeyVaultManagementClient(this.credentials, subscriptionID); | ||
| const vault = await keyVaultClient.vaults.get(keyVaultConfig.resourceGroup, keyVaultConfig.name).catch((e) => {throw new Error("Error: Specified vault not found")}); |
There was a problem hiding this comment.
Is the .catch() required here? If you are using async/await it should throw an exception that you can catch with normal catch block.
| @@ -58,6 +58,9 @@ export class FunctionAppResource implements ArmResourceTemplateGenerator { | |||
| "apiVersion": "2016-03-01", | |||
| "name": "[parameters('functionAppName')]", | |||
| "location": "[parameters('location')]", | |||
| "identity": { | |||
There was a problem hiding this comment.
Is this always going to be the case? Is there any situation where someone would not want system assigned identity?
There was a problem hiding this comment.
I am not sure, I don't know if there is any issue with always assigning the identity
* Add key vault linking * Add keyvault service * Update tests * Fix test * Add tests for coverage and address more pr feedback * Address final pr comments
|
How can we use it in a serverless.yml file? Is there any example for the topic? |
It is bad practice to push code with secrets hard coded. Services such as keyvault allow users to securely store their keys and just reference them from their code. This change adds sets up a managed identity for the function app and sets permissions to allow the serverless app to get secrets.
AB#535