CompositionalIT / farmer

Repeatable Azure deployments with ARM templates - made easy!
https://compositionalit.github.io/farmer
MIT License
523 stars 157 forks source link

Maintenance: Updating Azure API versions to latest #987

Open Tom-Sloboda opened 1 year ago

Tom-Sloboda commented 1 year ago

Cleaning up dev code

This PR closes #

The changes in this PR are as follows:

I have read the contributing guidelines and have completed the following:

If I haven't completed any of the tasks above, I include the reasons why here:

ninjarobot commented 1 year ago

We normally update these when in need of a new feature from an API, as sometimes new API versions ship with breaking changes to behaviors. Are there certain API versions you need updated to get a new feature or is this just a general update to latest versions?

martinbryant commented 1 year ago

@isaacabraham can you have a look at reviewing this please?

isaacabraham commented 5 months ago

The PR is so old now, the main codebase has moved on. It needs to be brought up (merged in) and then end-to-end tested before it can be accepted, I'm afraid.

I know this isn't great - it should have been addressed ages ago. It's partly because it didn't solve a specific issue that had been raised that we didn't look at it.

isaacabraham commented 5 months ago

Just to add to that - if there are no specific issues / requirements that this PR fixes, I'm inclined to close it - changing versions like this does present a (small) risk.

ninjarobot commented 5 months ago

They are usually safe, and especially in the last 2 years the ARM team has really cracked down on breaking changes. But I've definitely run into unexpected issues where a resource changes default behavior in newer API versions. One that got our team was that by switching the API version, the default zone behavior changed (older version didn't have zone support) and this led to unexpected deployment errors.

I'm generally in favor of using the newest versions, just usually when someone does them in a PR, they are actively testing with that resource. I'd like us to make sure we are testing any version we upgrade with an actual ARM deployment and making sure we end up with the expected resources.

That said, some versions are old enough to be deprecated, so we may need to embark on this effort in a coordinated manner, or at least guide contributors to use the latest versions in the PR's so we are moving in that direction. I can think of three resources - classic AppInsights, single-server Postgres, and basic public IPs - that are very near deprecation as both an API and a resource.