diff --git a/deploy/crds/crd-challenges.yaml b/deploy/crds/crd-challenges.yaml index 058e97ae7..605d8a222 100644 --- a/deploy/crds/crd-challenges.yaml +++ b/deploy/crds/crd-challenges.yaml @@ -205,8 +205,12 @@ spec: - subscriptionID properties: clientID: + description: if both this and ClientSecret are left unset + MSI will be used type: string clientSecretSecretRef: + description: if both this and ClientID are left unset MSI + will be used type: object required: - name @@ -233,6 +237,8 @@ spec: subscriptionID: type: string tenantID: + description: when specifying ClientID and ClientSecret then + this field is also needed type: string clouddns: description: ACMEIssuerDNS01ProviderCloudDNS is a structure diff --git a/deploy/crds/crd-clusterissuers.yaml b/deploy/crds/crd-clusterissuers.yaml index e8ff79e39..b2fdab330 100644 --- a/deploy/crds/crd-clusterissuers.yaml +++ b/deploy/crds/crd-clusterissuers.yaml @@ -248,8 +248,12 @@ spec: - subscriptionID properties: clientID: + description: if both this and ClientSecret are left + unset MSI will be used type: string clientSecretSecretRef: + description: if both this and ClientID are left unset + MSI will be used type: object required: - name @@ -278,6 +282,8 @@ spec: subscriptionID: type: string tenantID: + description: when specifying ClientID and ClientSecret + then this field is also needed type: string clouddns: description: ACMEIssuerDNS01ProviderCloudDNS is a structure diff --git a/deploy/crds/crd-issuers.yaml b/deploy/crds/crd-issuers.yaml index 6d62939a7..322d75169 100644 --- a/deploy/crds/crd-issuers.yaml +++ b/deploy/crds/crd-issuers.yaml @@ -248,8 +248,12 @@ spec: - subscriptionID properties: clientID: + description: if both this and ClientSecret are left + unset MSI will be used type: string clientSecretSecretRef: + description: if both this and ClientID are left unset + MSI will be used type: object required: - name @@ -278,6 +282,8 @@ spec: subscriptionID: type: string tenantID: + description: when specifying ClientID and ClientSecret + then this field is also needed type: string clouddns: description: ACMEIssuerDNS01ProviderCloudDNS is a structure diff --git a/pkg/apis/acme/v1alpha2/types_issuer.go b/pkg/apis/acme/v1alpha2/types_issuer.go index 5450f20c3..c0fa7c2d7 100644 --- a/pkg/apis/acme/v1alpha2/types_issuer.go +++ b/pkg/apis/acme/v1alpha2/types_issuer.go @@ -343,12 +343,19 @@ type ACMEIssuerDNS01ProviderRoute53 struct { // ACMEIssuerDNS01ProviderAzureDNS is a structure containing the // configuration for Azure DNS type ACMEIssuerDNS01ProviderAzureDNS struct { + + // if both this and ClientSecret are left unset MSI will be used + // +optional ClientID string `json:"clientID,omitempty"` + // if both this and ClientID are left unset MSI will be used + // +optional ClientSecret *cmmeta.SecretKeySelector `json:"clientSecretSecretRef,omitempty"` SubscriptionID string `json:"subscriptionID"` + // when specifying ClientID and ClientSecret then this field is also needed + // +optional TenantID string `json:"tenantID,omitempty"` ResourceGroupName string `json:"resourceGroupName"` diff --git a/pkg/apis/acme/v1alpha3/types_issuer.go b/pkg/apis/acme/v1alpha3/types_issuer.go index 85b9a2c18..4a72162ae 100644 --- a/pkg/apis/acme/v1alpha3/types_issuer.go +++ b/pkg/apis/acme/v1alpha3/types_issuer.go @@ -343,12 +343,19 @@ type ACMEIssuerDNS01ProviderRoute53 struct { // ACMEIssuerDNS01ProviderAzureDNS is a structure containing the // configuration for Azure DNS type ACMEIssuerDNS01ProviderAzureDNS struct { + + // if both this and ClientSecret are left unset MSI will be used + // +optional ClientID string `json:"clientID,omitempty"` + // if both this and ClientID are left unset MSI will be used + // +optional ClientSecret *cmmeta.SecretKeySelector `json:"clientSecretSecretRef,omitempty"` SubscriptionID string `json:"subscriptionID"` + // when specifying ClientID and ClientSecret then this field is also needed + // +optional TenantID string `json:"tenantID,omitempty"` ResourceGroupName string `json:"resourceGroupName"` diff --git a/pkg/internal/apis/certmanager/validation/issuer.go b/pkg/internal/apis/certmanager/validation/issuer.go index df07c012f..a65acb13b 100644 --- a/pkg/internal/apis/certmanager/validation/issuer.go +++ b/pkg/internal/apis/certmanager/validation/issuer.go @@ -253,18 +253,17 @@ func ValidateACMEChallengeSolverDNS01(p *cmacme.ACMEChallengeSolverDNS01, fldPat el = append(el, field.Forbidden(fldPath.Child("azuredns"), "may not specify more than one provider type")) } else { numProviders++ - // if ClientID is defined then clientSecret and tenantID must be defined - if len(p.AzureDNS.ClientID) > 0 { - el = append(el, ValidateSecretKeySelector(p.AzureDNS.ClientSecret, fldPath.Child("azuredns", "clientSecretSecretRef"))...) - if len(p.AzureDNS.TenantID) == 0 { - el = append(el, field.Required(fldPath.Child("azuredns", "tenantID"), "")) - } - } - // if ClientSecret is defined then both ClientID and TenantID must be defined - if p.AzureDNS.ClientSecret != nil { + // if ClientID or ClientSecret or TenantID are defined then all of ClientID, ClientSecret and tenantID must be defined + // We check things separately because + if len(p.AzureDNS.ClientID) > 0 || len(p.AzureDNS.TenantID) > 0 || p.AzureDNS.ClientSecret != nil { if len(p.AzureDNS.ClientID) == 0 { el = append(el, field.Required(fldPath.Child("azuredns", "clientID"), "")) } + if p.AzureDNS.ClientSecret == nil { + el = append(el, field.Required(fldPath.Child("azuredns", "clientSecretSecretRef"), "")) + } else { + el = append(el, ValidateSecretKeySelector(p.AzureDNS.ClientSecret, fldPath.Child("azuredns", "clientSecretSecretRef"))...) + } if len(p.AzureDNS.TenantID) == 0 { el = append(el, field.Required(fldPath.Child("azuredns", "tenantID"), "")) } diff --git a/pkg/internal/apis/certmanager/validation/issuer_test.go b/pkg/internal/apis/certmanager/validation/issuer_test.go index 8776d1c1c..76bf22e4d 100644 --- a/pkg/internal/apis/certmanager/validation/issuer_test.go +++ b/pkg/internal/apis/certmanager/validation/issuer_test.go @@ -607,6 +607,133 @@ func TestValidateACMEIssuerDNS01Config(t *testing.T) { "must be either empty or one of AzurePublicCloud, AzureChinaCloud, AzureGermanCloud or AzureUSGovernmentCloud"), }, }, + "invalid azuredns missing clientSecret and tenantID": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + ClientID: "some-client-id", + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientSecretSecretRef"), ""), + field.Required(fldPath.Child("azuredns", "tenantID"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns missing clientID and tenantID": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + ClientSecret: &cmmeta.SecretKeySelector{ + Key: "some-key", + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: "some-secret-name", + }, + }, + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientID"), ""), + field.Required(fldPath.Child("azuredns", "tenantID"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns missing clientID and clientSecret": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + TenantID: "some-tenant-id", + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientID"), ""), + field.Required(fldPath.Child("azuredns", "clientSecretSecretRef"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns missing clientID": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + ClientSecret: &cmmeta.SecretKeySelector{ + Key: "some-key", + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: "some-secret-name", + }, + }, + TenantID: "some-tenant-id", + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientID"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns missing clientSecret": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + TenantID: "some-tenant-id", + ClientID: "some-client-id", + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientSecretSecretRef"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns clientSecret missing key": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + TenantID: "some-tenant-id", + ClientID: "some-client-id", + ClientSecret: &cmmeta.SecretKeySelector{ + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: "some-secret-name", + }, + }, + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientSecretSecretRef", "key"), "secret key is required"), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns clientSecret missing secret name": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + TenantID: "some-tenant-id", + ClientID: "some-client-id", + ClientSecret: &cmmeta.SecretKeySelector{ + Key: "some-key", + }, + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "clientSecretSecretRef", "name"), "secret name is required"), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, + "invalid azuredns missing tenantID": { + cfg: &cmacme.ACMEChallengeSolverDNS01{ + AzureDNS: &cmacme.ACMEIssuerDNS01ProviderAzureDNS{ + ClientID: "some-client-id", + ClientSecret: &cmmeta.SecretKeySelector{ + Key: "some-key", + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: "some-secret-name", + }, + }, + }, + }, + errs: []*field.Error{ + field.Required(fldPath.Child("azuredns", "tenantID"), ""), + field.Required(fldPath.Child("azuredns", "subscriptionID"), ""), + field.Required(fldPath.Child("azuredns", "resourceGroupName"), ""), + }, + }, "missing akamai config": { cfg: &cmacme.ACMEChallengeSolverDNS01{ Akamai: &cmacme.ACMEIssuerDNS01ProviderAkamai{}, diff --git a/pkg/issuer/acme/dns/azuredns/azuredns.go b/pkg/issuer/acme/dns/azuredns/azuredns.go index 9d55fc299..7e91c5c3b 100644 --- a/pkg/issuer/acme/dns/azuredns/azuredns.go +++ b/pkg/issuer/acme/dns/azuredns/azuredns.go @@ -13,7 +13,6 @@ package azuredns import ( "context" "fmt" - "os" "strings" "k8s.io/klog" @@ -35,22 +34,6 @@ type DNSProvider struct { zoneName string } -// NewDNSProvider returns a DNSProvider instance configured for the Azure -// DNS service. -// Credentials are automatically detected from environment variables -func NewDNSProvider(dns01Nameservers []string) (*DNSProvider, error) { - - clientID := os.Getenv("AZURE_CLIENT_ID") - clientSecret := os.Getenv("AZURE_CLIENT_SECRET") - subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID") - tenantID := os.Getenv("AZURE_TENANT_ID") - resourceGroupName := ("AZURE_RESOURCE_GROUP") - zoneName := ("AZURE_ZONE_NAME") - environment := ("AZURE_ENVIRONMENT") - - return NewDNSProviderCredentials(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, zoneName, dns01Nameservers, true) -} - // NewDNSProviderCredentials returns a DNSProvider instance configured for the Azure // DNS service using static credentials from its parameters func NewDNSProviderCredentials(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, zoneName string, dns01Nameservers []string, ambient bool) (*DNSProvider, error) { @@ -62,31 +45,10 @@ func NewDNSProviderCredentials(environment, clientID, clientSecret, subscription return nil, err } } - spt := &adal.ServicePrincipalToken{} - if clientID != "" { - klog.Info("azuredns authenticating with clientID and secret key") - oauthConfig, err := adal.NewOAuthConfig(env.ActiveDirectoryEndpoint, tenantID) - if err != nil { - return nil, err - } - spt, err = adal.NewServicePrincipalToken(*oauthConfig, clientID, clientSecret, env.ResourceManagerEndpoint) - if err != nil { - return nil, err - } - } else { - klog.Info("azuredns authenticating with managed identity") - if !ambient { - return nil, fmt.Errorf("ClientID is not set but neither `--cluster-issuer-ambient-credentials` nor `--issuer-ambient-credentials` are set. These are necessary to enable Azure Managed Identities") - } - msiEndpoint, err := adal.GetMSIVMEndpoint() - if err != nil { - return nil, fmt.Errorf("failed to get the managed service identity endpoint: %v", err) - } - spt, err = adal.NewServicePrincipalTokenFromMSI(msiEndpoint, env.ServiceManagementEndpoint) - if err != nil { - return nil, fmt.Errorf("failed to create the managed service identity token: %v", err) - } + spt, err := getAuthorization(env, clientID, clientSecret, subscriptionID, tenantID, ambient) + if err != nil { + return nil, err } rc := dns.NewRecordSetsClientWithBaseURI(env.ResourceManagerEndpoint, subscriptionID) @@ -104,6 +66,35 @@ func NewDNSProviderCredentials(environment, clientID, clientSecret, subscription }, nil } +func getAuthorization(env azure.Environment, clientID, clientSecret, subscriptionID, tenantID string, ambient bool) (*adal.ServicePrincipalToken, error) { + if clientID != "" { + klog.Info("azuredns authenticating with clientID and secret key") + oauthConfig, err := adal.NewOAuthConfig(env.ActiveDirectoryEndpoint, tenantID) + if err != nil { + return nil, err + } + spt, err := adal.NewServicePrincipalToken(*oauthConfig, clientID, clientSecret, env.ResourceManagerEndpoint) + if err != nil { + return nil, err + } + return spt, nil + } + klog.Info("No ClientID found: authenticating azuredns with managed identity (MSI)") + if !ambient { + return nil, fmt.Errorf("ClientID is not set but neither `--cluster-issuer-ambient-credentials` nor `--issuer-ambient-credentials` are set. These are necessary to enable Azure Managed Identities") + } + msiEndpoint, err := adal.GetMSIVMEndpoint() + if err != nil { + return nil, fmt.Errorf("failed to get the managed service identity endpoint: %v", err) + } + + spt, err := adal.NewServicePrincipalTokenFromMSI(msiEndpoint, env.ServiceManagementEndpoint) + if err != nil { + return nil, fmt.Errorf("failed to create the managed service identity token: %v", err) + } + return spt, nil +} + // Present creates a TXT record using the specified parameters func (c *DNSProvider) Present(domain, fqdn, value string) error { return c.createRecord(fqdn, value, 60) diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index 13d0cb699..4ff6a9f9c 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -331,6 +331,8 @@ func (s *Solver) solverForChallenge(ctx context.Context, issuer v1alpha2.Generic case providerConfig.AzureDNS != nil: dbg.Info("preparing to create AzureDNS provider") secret := "" + // if ClientID is empty, then we try to use MSI (azure metadata API for credentials) + // if ClientID is empty we don't even try to get the ClientSecret because it would not be used if providerConfig.AzureDNS.ClientID != "" { clientSecret, err := s.secretLister.Secrets(resourceNamespace).Get(providerConfig.AzureDNS.ClientSecret.Name) if err != nil {