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

Fix ServiceBus Namespace Slug #136

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GirishMaddineni
Copy link

@GirishMaddineni GirishMaddineni commented Aug 15, 2024

This is the PR for fixing ServiceBus Namespace slug coming from naming module, when using module.naming.servicebus_namespace.name it is throwing an error stating,

Error: "name" cannot end with a hyphen, -sb, or -mgmt │
│ with module.service_bus.azurerm_servicebus_namespace.this,
│ on .terraform/modules/service_bus/main.tf line 3, in resource "azurerm_servicebus_namespace" "this":
│ 3: name = var.name

current Behaviour:
-sb

Expected Behaviour to resolve issue
-sbus

Reference documentation:
https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-quickstart-topics-subscriptions-portal

image

the servicebus namespace slug coming from naming module when using module.naming.servicebus_namespace.name is not allowing by the Azure when creating an resource, with an error stating, "name" cannot end with a hyphen, -sb, or -mgmt
│ 
│   with module.service_bus.azurerm_servicebus_namespace.this,
│   on .terraform/modules/service_bus/main.tf line 3, in resource "azurerm_servicebus_namespace" "this":
│    3:   name                          = var.name
@GirishMaddineni
Copy link
Author

@microsoft-github-policy-service agree

@GirishMaddineni
Copy link
Author

@Nepomuceno @shanselman @alloy, can I please get a review on this Issue/PR. it's a blocker for our current work.

@Nepomuceno
Copy link
Member

HI @Girish-Maddineni thx for your contribution.

This is breaking change so we unfortunatly cannot approve this change anyone that has deployed a service bus namespace using this module would have it to fail.

you can add a suffix to it so it would not have -sb in the end. I suggest creating a new call to the naming module if you want the suffix to apply just to this.

Also just for the future there is a need to generate the files after you change the definition you can so that by running
make all

@GirishMaddineni
Copy link
Author

Thank you @Nepomuceno. I ran the Make all command and Updated PR with latest changes from it. I will make a note to run it in the future.

@davenicoll
Copy link

davenicoll commented Aug 20, 2024

This is breaking change so we unfortunatly cannot approve this change anyone that has deployed a service bus namespace using this module would have it to fail.

you can add a suffix to it so it would not have -sb in the end. I suggest creating a new call to the naming module if you want the suffix to apply just to this.

@Nepomuceno is it a breaking change, if with the currently implemented slug, servicebus namespace can not be deployed? surely everyone has had to workaround this?

can you advise what the next steps are for this PR please? thanks

@Nepomuceno
Copy link
Member

@davenicoll This is a breaking change because if someone created a service bus and they are using suffix this would work if we do change this it would recreate their resources.

@jamengual
Copy link

Every single change is a breaking change in this module. (based on your comments on other PRs)
Why can't you guys create a major release to support YOUR OWN resource naming standards?
People should tag the version on the module instantiation, so this change should not be considered a breaking change.
Since the module's core goal is to support compatibility with the naming standards of Azure resources, I do not understand why these changes are so difficult to merge.

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.

4 participants