From d4d49140a6c0d60ae52aad4d0188ceb9ebfce4fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=CC=81s=CC=8C=20Freund?= Date: Fri, 13 Aug 2021 17:16:12 +0200 Subject: [PATCH] resolve null dereference issues by passing the whole struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomáš Freund --- pkg/issuer/acme/dns/azuredns/BUILD.bazel | 2 ++ pkg/issuer/acme/dns/azuredns/azuredns.go | 16 ++++++++++------ pkg/issuer/acme/dns/azuredns/azuredns_test.go | 9 +++++---- pkg/issuer/acme/dns/dns.go | 5 ++--- pkg/issuer/acme/dns/util_test.go | 4 ++-- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/pkg/issuer/acme/dns/azuredns/BUILD.bazel b/pkg/issuer/acme/dns/azuredns/BUILD.bazel index 08a9cbbb7..229bd2b04 100644 --- a/pkg/issuer/acme/dns/azuredns/BUILD.bazel +++ b/pkg/issuer/acme/dns/azuredns/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/pkg/issuer/acme/dns/azuredns", visibility = ["//visibility:public"], deps = [ + "//pkg/apis/acme/v1:go_default_library", "//pkg/issuer/acme/dns/util:go_default_library", "//pkg/logs:go_default_library", "@com_github_azure_azure_sdk_for_go//services/dns/mgmt/2017-10-01/dns:go_default_library", @@ -22,6 +23,7 @@ go_test( srcs = ["azuredns_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/apis/acme/v1:go_default_library", "//pkg/issuer/acme/dns/util:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", ], diff --git a/pkg/issuer/acme/dns/azuredns/azuredns.go b/pkg/issuer/acme/dns/azuredns/azuredns.go index a65de26ea..bef7a90e8 100644 --- a/pkg/issuer/acme/dns/azuredns/azuredns.go +++ b/pkg/issuer/acme/dns/azuredns/azuredns.go @@ -23,6 +23,7 @@ import ( "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" + cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" "github.com/jetstack/cert-manager/pkg/issuer/acme/dns/util" logf "github.com/jetstack/cert-manager/pkg/logs" ) @@ -39,7 +40,7 @@ type DNSProvider struct { // 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, managedIdentityClientID string, managedIdentityResourceID string) (*DNSProvider, error) { +func NewDNSProviderCredentials(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, zoneName string, dns01Nameservers []string, ambient bool, managedIdentity *cmacme.AzureManagedIdentity) (*DNSProvider, error) { env := azure.PublicCloud if environment != "" { var err error @@ -49,7 +50,7 @@ func NewDNSProviderCredentials(environment, clientID, clientSecret, subscription } } - spt, err := getAuthorization(env, clientID, clientSecret, subscriptionID, tenantID, ambient, managedIdentityClientID, managedIdentityResourceID) + spt, err := getAuthorization(env, clientID, clientSecret, subscriptionID, tenantID, ambient, managedIdentity) if err != nil { return nil, err } @@ -70,7 +71,7 @@ func NewDNSProviderCredentials(environment, clientID, clientSecret, subscription }, nil } -func getAuthorization(env azure.Environment, clientID, clientSecret, subscriptionID, tenantID string, ambient bool, managedIdentityClientID string, managedIdentityResourceID string) (*adal.ServicePrincipalToken, error) { +func getAuthorization(env azure.Environment, clientID, clientSecret, subscriptionID, tenantID string, ambient bool, managedIdentity *cmacme.AzureManagedIdentity) (*adal.ServicePrincipalToken, error) { if clientID != "" { logf.Log.V(logf.InfoLevel).Info("azuredns authenticating with clientID and secret key") oauthConfig, err := adal.NewOAuthConfig(env.ActiveDirectoryEndpoint, tenantID) @@ -88,10 +89,13 @@ func getAuthorization(env azure.Environment, clientID, clientSecret, subscriptio 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") } - opt := adal.ManagedIdentityOptions{ - ClientID: managedIdentityClientID, - IdentityResourceID: managedIdentityResourceID, + opt := adal.ManagedIdentityOptions{} + + if managedIdentity != nil { + opt.ClientID = managedIdentity.ClientID + opt.IdentityResourceID = managedIdentity.ResourceID } + spt, err := adal.NewServicePrincipalTokenFromManagedIdentity(env.ServiceManagementEndpoint, &opt) if err != nil { return nil, fmt.Errorf("failed to create the managed service identity token: %v", err) diff --git a/pkg/issuer/acme/dns/azuredns/azuredns_test.go b/pkg/issuer/acme/dns/azuredns/azuredns_test.go index d995c7044..275a9c71c 100644 --- a/pkg/issuer/acme/dns/azuredns/azuredns_test.go +++ b/pkg/issuer/acme/dns/azuredns/azuredns_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + v1 "github.com/jetstack/cert-manager/pkg/apis/acme/v1" "github.com/jetstack/cert-manager/pkg/issuer/acme/dns/util" "github.com/stretchr/testify/assert" ) @@ -45,7 +46,7 @@ func TestLiveAzureDnsPresent(t *testing.T) { if !azureLiveTest { t.Skip("skipping live test") } - provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, "", "") + provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.NoError(t, err) err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "123d==") @@ -59,7 +60,7 @@ func TestLiveAzureDnsCleanUp(t *testing.T) { time.Sleep(time.Second * 5) - provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, "", "") + provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.NoError(t, err) err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "123d==") @@ -69,10 +70,10 @@ func TestLiveAzureDnsCleanUp(t *testing.T) { func TestInvalidAzureDns(t *testing.T) { validEnv := []string{"", "AzurePublicCloud", "AzureChinaCloud", "AzureGermanCloud", "AzureUSGovernmentCloud"} for _, env := range validEnv { - _, err := NewDNSProviderCredentials(env, "cid", "secret", "", "", "", "", util.RecursiveNameservers, false, "", "") + _, err := NewDNSProviderCredentials(env, "cid", "secret", "", "", "", "", util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.NoError(t, err) } - _, err := NewDNSProviderCredentials("invalid env", "cid", "secret", "", "", "", "", util.RecursiveNameservers, false, "", "") + _, err := NewDNSProviderCredentials("invalid env", "cid", "secret", "", "", "", "", util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.Error(t, err) } diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index af51ea797..e87d906d7 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -60,7 +60,7 @@ type dnsProviderConstructors struct { cloudDNS func(project string, serviceAccount []byte, dns01Nameservers []string, ambient bool, hostedZoneName string) (*clouddns.DNSProvider, error) cloudFlare func(email, apikey, apiToken string, dns01Nameservers []string) (*cloudflare.DNSProvider, error) route53 func(accessKey, secretKey, hostedZoneID, region, role string, ambient bool, dns01Nameservers []string) (*route53.DNSProvider, error) - azureDNS func(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName string, dns01Nameservers []string, ambient bool, managedIdentityClientID string, managedIdentityResourceID string) (*azuredns.DNSProvider, error) + azureDNS func(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName string, dns01Nameservers []string, ambient bool, managedIdentity *cmacme.AzureManagedIdentity) (*azuredns.DNSProvider, error) acmeDNS func(host string, accountJson []byte, dns01Nameservers []string) (*acmedns.DNSProvider, error) digitalOcean func(token string, dns01Nameservers []string) (*digitalocean.DNSProvider, error) } @@ -345,8 +345,7 @@ func (s *Solver) solverForChallenge(ctx context.Context, issuer v1.GenericIssuer providerConfig.AzureDNS.HostedZoneName, s.DNS01Nameservers, canUseAmbientCredentials, - providerConfig.AzureDNS.ManagedIdentity.ClientID, - providerConfig.AzureDNS.ManagedIdentity.ResourceID, + providerConfig.AzureDNS.ManagedIdentity, ) if err != nil { return nil, nil, fmt.Errorf("error instantiating azuredns challenge solver: %s", err) diff --git a/pkg/issuer/acme/dns/util_test.go b/pkg/issuer/acme/dns/util_test.go index 67cf503a4..0dac00f47 100644 --- a/pkg/issuer/acme/dns/util_test.go +++ b/pkg/issuer/acme/dns/util_test.go @@ -143,8 +143,8 @@ func newFakeDNSProviders() *fakeDNSProviders { f.call("route53", accessKey, secretKey, hostedZoneID, region, role, ambient, util.RecursiveNameservers) return nil, nil }, - azureDNS: func(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName string, dns01Nameservers []string, ambient bool, managedIdentityClientID string, managedIdentityResourceID string) (*azuredns.DNSProvider, error) { - f.call("azuredns", clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName, util.RecursiveNameservers, ambient, "", "") + azureDNS: func(environment, clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName string, dns01Nameservers []string, ambient bool, managedIdentity *cmacme.AzureManagedIdentity) (*azuredns.DNSProvider, error) { + f.call("azuredns", clientID, clientSecret, subscriptionID, tenantID, resourceGroupName, hostedZoneName, util.RecursiveNameservers, ambient, managedIdentity) return nil, nil }, acmeDNS: func(host string, accountJson []byte, dns01Nameservers []string) (*acmedns.DNSProvider, error) {