addressed all reviewes and added all validation tests

Signed-off-by: gitirabassi <giacomo@tirabassi.eu>
This commit is contained in:
gitirabassi 2020-03-26 11:30:16 +01:00
parent 7595707d16
commit 7a9788adba
No known key found for this signature in database
GPG Key ID: 2E88FEA935FC6CFE
9 changed files with 201 additions and 50 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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"`

View File

@ -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"`

View File

@ -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"), ""))
}

View File

@ -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{},

View File

@ -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)

View File

@ -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 {