-
Notifications
You must be signed in to change notification settings - Fork 432
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
create local network gateway resource #1457
create local network gateway resource #1457
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thank you for opening your Pull Request ! 🙂 While a Tailwarden team member takes a look at your PR we would like to invite you to join our official Discord server, where you can interact directly with other contributors and Tailwarden team members. Link here: https://discord.tailwarden.com
|
||
for _, resourceGroup := range page.ResourceGroupListResult.Value { | ||
|
||
resourceGroupNames = append(resourceGroupNames, *resourceGroup.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add other details like https://github.com/tailwarden/komiser/pull/1457/files#diff-e34e11157def2d0cf86866b6de46e377bda2def7d4aa338041dac225c5a1e7cfR56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the change to add the resource as a model and not the string and fill the details.
However I do not added to to listOfSupportedServices as a resource group is not a provider.
Name: *lng.Name, | ||
FetchedAt: time.Now(), | ||
Tags: tags, | ||
Link: fmt.Sprint("https://portal.azure.com/#resource%s", *lng.ID), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we fix the failure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than this nit, LGTM
dashboard/utils/serviceHelper.ts
Outdated
@@ -87,7 +87,8 @@ export const allProvidersServices: { [key in Providers]: string[] } = { | |||
'firewall', | |||
'load balancer', | |||
'databox', | |||
'queue' | |||
'queue', | |||
'Local Network Gateway' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Problem
Solving Issue #396
Solution
Adding resource
Changes Made
added local network gateway
also created a function to get the resource groups in the subscription to be able to call all the local network gateways, as function do not call under a subscription level
How to Test
Added a new local network gateway in the Azure subscription, test if it shows successfully in Komiser. Cost is not showing, as LNGs do not generate cost.
Screenshots
Notes
[Any additional notes or information that you would like to share with the reviewers.]
Checklist
Reviewers
@jakepage91