Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly resolve template file from parameter files when suffix appears multiple times in path #885

Merged

Conversation

Xitric
Copy link
Contributor

@Xitric Xitric commented May 31, 2024

Overview/Summary

Proposes to close #884

Currently, when AzOps resolves a template file from a parameter file, it uses two replace operations, one of which matches against the regex wildcard character ., rather than the literal character period (\.). Furthermore, by using two separate replace operations, the code misses a series of edge cases.

Consider a template file named templatexabc.xabc.bicep and a valid parameter file templatexabc.xabc.xabc.bicepparam. After performing the two replace operations, we are left with a template file name templat.bicep which doesn't exist.

This PR fixes/adds/changes/removes

  1. Changes how template file names are resolved from parameter files by ensuring that the suffix is matched at most once at the end of the parameter file name.

Breaking Changes

  1. This could theoretically break deployment for someone who relies on the suffix to be matched multiple times in the template path, but I don't think that was ever intended behaviour.

Let me know if this is an issue. I can modify the PR to maintain the two separate replace operations and simply escape the period . in the regular expression to solve the first half of #884. That would be sufficient for my team. Let me also know if we need to change the documentation to make the behaviour of the suffix clearer.

Testing Evidence

I added a test case to src/tests/integration/Repository.Tests.ps1. Haven't been able to run it yet, but I would expect it to work.

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.

@Xitric Xitric requested review from a team as code owners May 31, 2024 08:07
@Jefajers Jefajers self-assigned this Jun 3, 2024
@Jefajers Jefajers added triage bug Something isn't working and removed triage labels Jun 3, 2024
@Jefajers Jefajers added this to the v2.6.4 milestone Jun 3, 2024
@Xitric
Copy link
Contributor Author

Xitric commented Jun 5, 2024

The changes look good @Jefajers!

Copy link
Member

@Jefajers Jefajers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Jefajers Jefajers merged commit 882f748 into Azure:main Jun 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
2 participants