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

Updating to support latest Provider functions #6

Merged
merged 10 commits into from
Jul 16, 2024

Conversation

joeharlan
Copy link
Contributor

Description

This PR includes some changes in formatting for readability, but also include some functional changes. The main functional change includes code to support handling any empty role templates returned by the RSC API that the Azure RM provider interprets as a change on every plan/apply operation despite there being no actual changes.

Related Issue

This project only accepts pull requests related to open issues.

  • If suggesting a new feature or change, please discuss it in an issue first.
  • If fixing a bug, there should be an issue describing it with steps to reproduce

Please link to the issue here

Motivation and Context

The proposed changes solve the previously mentioned issue with the RG-scoped Azure SQL DB and Azure SQL MI role templates containing no permissions.

How Has This Been Tested?

  • Please describe in detail how you tested your changes.
  • Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc.

The changes were tested in the Oasis Labs environment using the 0.9.0-beta.8 polaris Provider.

Screenshots (if appropriate):

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Go over all the following points, and put an x in all the boxes that apply. If you're unsure about any of these, don't hesitate to ask. We're here to help!

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the CHANGELOG file accordingly for the version that this merge modifies.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Collaborator

@DamaniN DamaniN left a comment

Choose a reason for hiding this comment

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

@joeharlan, the "RubrikBackups-RG-DontDelete-terraform Resource Group name is based on what RSC uses. Changing this here will be a breaking change for any existing customer who is using this module. All resources in the RG will have to be removed and recreated, including backups. Alternatively, this RG name will need to be renamed in the customer's existing Terraform state file.

If you really want to make this change, can you include documentation about the breaking change? Also, please document how to rename the RGs in the existing state file before an update is run?

@joeharlan
Copy link
Contributor Author

joeharlan commented Jul 4, 2024

@DamaniN Reverted to previous name. Though I have to ask, is it common to use module examples as-is, or is it more common to clone them to your own repo and modify as desired?

Copy link
Collaborator

@DamaniN DamaniN left a comment

Choose a reason for hiding this comment

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

@joeharlan the names and descriptions provided for the roles match what RSC currently creates during the with and without OAuth workflows. Please confirm that you want to move away from this standard.

The changes you propose do make sense to remove the Polaris name, however, TF will now do something different than the UI. This may lead to confusion with support.

main.tf Show resolved Hide resolved
main.tf Show resolved Hide resolved
Copy link
Collaborator

@DamaniN DamaniN left a comment

Choose a reason for hiding this comment

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

LGTM

@DamaniN DamaniN merged commit 2194fe3 into rubrikinc:master Jul 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants