feat(new): Added Azure.AppService.MigrateNodeJs

Azure / PSRule.Rules.Azure

Rules to validate Azure resources and infrastructure as code (IaC) using PSRule.

https://azure.github.io/PSRule.Rules.Azure/

MIT License

389 stars 84 forks source link

feat(new): Added Azure.AppService.MigrateNodeJs #2889

Closed BenjaminEngeset closed 3 months ago

BenjaminEngeset commented 3 months ago

PR Summary

Working toward #2879

Added Azure.AppService.MigrateNodeJs.

PR Checklist

[x] PR has a meaningful title
[ ] Summarized changes
[ ] Change is not breaking
[ ] This PR is ready to merge and is not Work in Progress
Rule changes
- [ ] Unit tests created/ updated
- [ ] Rule documentation created/ updated
- [ ] Link to a filed issue
- [ ] Change log has been updated with change under unreleased section
Other code changes
- [ ] Unit tests created/ updated
- [ ] Link to a filed issue
- [ ] Change log has been updated with change under unreleased section

BenjaminEngeset commented 3 months ago

Hi @BernieWhite,

Could you please provide some feedback on the work-in-progress code, specifically regarding the rule design/body? I am interested in your thoughts.

BenjaminEngeset commented 3 months ago

Hi again @BernieWhite.

Can you give me some feedback?

I'm still missing the tests, but wanted to check in with you.

BenjaminEngeset commented 3 months ago

Hi @BenjaminEngeset, great progress.

From some additional research, if you are accessing app settings via the appsettings named config then the properties can be set via { }, otherwise they should be:
appSettings: [  
  {  
    name: 'WEBSITE_NODE_DEFAULT_VERSION'  
    value: '~20  
  }  
] 

Great catch! I appreciate you identifying this. I will add in logic for this.

BenjaminEngeset commented 3 months ago

So @BernieWhite, what do you think about the rule now?

I have added test scenarios for several cases, but I have not added cases for sub-resources configured within the parent resource. If you feel that is needed, let me know.

BernieWhite commented 3 months ago

So @BernieWhite, what do you think about the rule now?

I have added test scenarios for several cases, but I have not added cases for sub-resources configured within the parent resource. If you feel that is needed, let me know.

Thanks @BenjaminEngeset Looks look. Yes let's add at least one good and bad case for Microsoft.Web/sites sub-resources.

BenjaminEngeset commented 3 months ago

I have added a good and bad test case now @BernieWhite.

Following logic:

In the first case the app setting will override what was set within the parent resource.
In the second case the app setting will override what was set within the parent resource, but it will set what we expect as good.