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

feat: enable Azure subscription ID override #4186

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ func main() {
}
p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID, sd.New(awsSession))
case "azure-dns", "azure":
p, err = azure.NewAzureProvider(cfg.AzureConfigFile, domainFilter, zoneNameFilter, zoneIDFilter, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun)
p, err = azure.NewAzureProvider(cfg.AzureConfigFile, domainFilter, zoneNameFilter, zoneIDFilter, cfg.AzureSubscriptionID, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun)
case "azure-private-dns":
p, err = azure.NewAzurePrivateDNSProvider(cfg.AzureConfigFile, domainFilter, zoneIDFilter, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun)
p, err = azure.NewAzurePrivateDNSProvider(cfg.AzureConfigFile, domainFilter, zoneIDFilter, cfg.AzureSubscriptionID, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun)
case "bluecat":
p, err = bluecat.NewBluecatProvider(cfg.BluecatConfigFile, cfg.BluecatDNSConfiguration, cfg.BluecatDNSServerName, cfg.BluecatDNSDeployType, cfg.BluecatDNSView, cfg.BluecatGatewayHost, cfg.BluecatRootZone, cfg.TXTPrefix, cfg.TXTSuffix, domainFilter, zoneIDFilter, cfg.DryRun, cfg.BluecatSkipTLSVerify)
case "vinyldns":
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,8 @@ func (cfg *Config) ParseFlags(args []string) error {
app.Flag("aws-zone-match-parent", "Expand limit possible target by sub-domains (default: disabled)").BoolVar(&cfg.AWSZoneMatchParent)
app.Flag("aws-sd-service-cleanup", "When using the AWS CloudMap provider, delete empty Services without endpoints (default: disabled)").BoolVar(&cfg.AWSSDServiceCleanup)
app.Flag("azure-config-file", "When using the Azure provider, specify the Azure configuration file (required when --provider=azure)").Default(defaultConfig.AzureConfigFile).StringVar(&cfg.AzureConfigFile)
app.Flag("azure-resource-group", "When using the Azure provider, override the Azure resource group to use (required when --provider=azure-private-dns)").Default(defaultConfig.AzureResourceGroup).StringVar(&cfg.AzureResourceGroup)
app.Flag("azure-subscription-id", "When using the Azure provider, specify the Azure configuration file (required when --provider=azure-private-dns)").Default(defaultConfig.AzureSubscriptionID).StringVar(&cfg.AzureSubscriptionID)
app.Flag("azure-resource-group", "When using the Azure provider, override the Azure resource group to use (optional)").Default(defaultConfig.AzureResourceGroup).StringVar(&cfg.AzureResourceGroup)
app.Flag("azure-subscription-id", "When using the Azure provider, override the Azure subscription to use (optional)").Default(defaultConfig.AzureSubscriptionID).StringVar(&cfg.AzureSubscriptionID)
Copy link
Contributor Author

@pascalgn pascalgn Jan 16, 2024

Choose a reason for hiding this comment

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

Both flags are actually not required, because the value are taken from the config file by default, so I changed the help text here

app.Flag("azure-user-assigned-identity-client-id", "When using the Azure provider, override the client id of user assigned identity in config file (optional)").Default("").StringVar(&cfg.AzureUserAssignedIdentityClientID)
app.Flag("tencent-cloud-config-file", "When using the Tencent Cloud provider, specify the Tencent Cloud configuration file (required when --provider=tencentcloud)").Default(defaultConfig.TencentCloudConfigFile).StringVar(&cfg.TencentCloudConfigFile)
app.Flag("tencent-cloud-zone-type", "When using the Tencent Cloud provider, filter for zones with visibility (optional, options: public, private)").Default(defaultConfig.TencentCloudZoneType).EnumVar(&cfg.TencentCloudZoneType, "", "public", "private")
Expand Down
4 changes: 2 additions & 2 deletions provider/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ type AzureProvider struct {
// NewAzureProvider creates a new Azure provider.
//
// Returns the provider or an error if a provider could not be created.
func NewAzureProvider(configFile string, domainFilter endpoint.DomainFilter, zoneNameFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, resourceGroup string, userAssignedIdentityClientID string, dryRun bool) (*AzureProvider, error) {
cfg, err := getConfig(configFile, resourceGroup, userAssignedIdentityClientID)
func NewAzureProvider(configFile string, domainFilter endpoint.DomainFilter, zoneNameFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, subscriptionID string, resourceGroup string, userAssignedIdentityClientID string, dryRun bool) (*AzureProvider, error) {
cfg, err := getConfig(configFile, subscriptionID, resourceGroup, userAssignedIdentityClientID)
if err != nil {
return nil, fmt.Errorf("failed to read Azure config file '%s': %v", configFile, err)
}
Expand Down
4 changes: 2 additions & 2 deletions provider/azure/azure_private_dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ type AzurePrivateDNSProvider struct {
// NewAzurePrivateDNSProvider creates a new Azure Private DNS provider.
//
// Returns the provider or an error if a provider could not be created.
func NewAzurePrivateDNSProvider(configFile string, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, resourceGroup, userAssignedIdentityClientID string, dryRun bool) (*AzurePrivateDNSProvider, error) {
cfg, err := getConfig(configFile, resourceGroup, userAssignedIdentityClientID)
func NewAzurePrivateDNSProvider(configFile string, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, subscriptionID string, resourceGroup string, userAssignedIdentityClientID string, dryRun bool) (*AzurePrivateDNSProvider, error) {
cfg, err := getConfig(configFile, subscriptionID, resourceGroup, userAssignedIdentityClientID)
if err != nil {
return nil, fmt.Errorf("failed to read Azure config file '%s': %v", configFile, err)
}
Expand Down
7 changes: 5 additions & 2 deletions provider/azure/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type config struct {
UserAssignedIdentityID string `json:"userAssignedIdentityID" yaml:"userAssignedIdentityID"`
}

func getConfig(configFile, resourceGroup, userAssignedIdentityClientID string) (*config, error) {
func getConfig(configFile, subscriptionID, resourceGroup, userAssignedIdentityClientID string) (*config, error) {
contents, err := os.ReadFile(configFile)
if err != nil {
return nil, fmt.Errorf("failed to read Azure config file '%s': %v", configFile, err)
Expand All @@ -53,7 +53,10 @@ func getConfig(configFile, resourceGroup, userAssignedIdentityClientID string) (
if err != nil {
return nil, fmt.Errorf("failed to read Azure config file '%s': %v", configFile, err)
}

// If a subscription ID was given, override what was present in the config file
if subscriptionID != "" {
cfg.SubscriptionID = subscriptionID
}
// If a resource group was given, override what was present in the config file
if resourceGroup != "" {
cfg.ResourceGroup = resourceGroup
Expand Down
14 changes: 14 additions & 0 deletions provider/azure/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ limitations under the License.
package azure

import (
"path"
"runtime"
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/stretchr/testify/assert"
)

func TestGetCloudConfiguration(t *testing.T) {
Expand All @@ -44,3 +47,14 @@ func TestGetCloudConfiguration(t *testing.T) {
})
}
}

func TestOverrideConfiguration(t *testing.T) {
_, filename, _, _ := runtime.Caller(0)
configFile := path.Join(path.Dir(filename), "config_test.json")
cfg, err := getConfig(configFile, "subscription-override", "rg-override", "")
if err != nil {
t.Errorf("got unexpected err %v", err)
}
assert.Equal(t, cfg.SubscriptionID, "subscription-override")
assert.Equal(t, cfg.ResourceGroup, "rg-override")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this test is useful/wanted, can also be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

It can help to ensures that the override still work when someone else makes a PR

7 changes: 7 additions & 0 deletions provider/azure/config_test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"tenantId": "tenant",
"subscriptionId": "subscription",
"resourceGroup": "rg",
"aadClientId": "clientId",
"aadClientSecret": "clientSecret"
}
Loading