From 124263794194b41f8d5942a15bb9704a90069fa0 Mon Sep 17 00:00:00 2001 From: tariqibrahim Date: Wed, 3 Oct 2018 17:19:17 -0600 Subject: [PATCH 1/4] Move utility methods to the helper package --- cmd/dcos-upgrade.go | 3 +- cmd/deploy.go | 2 +- cmd/scale.go | 2 +- cmd/upgrade.go | 2 +- pkg/acsengine/defaults.go | 78 ++++++++++++------------- pkg/acsengine/engine_test.go | 62 ++++++++++++++++++++ pkg/acsengine/output.go | 3 +- pkg/acsengine/ssh.go | 32 ---------- pkg/acsengine/ssh_test.go | 29 --------- pkg/{acsengine => helpers}/filesaver.go | 2 +- pkg/helpers/helpers.go | 23 ++++++++ pkg/helpers/helpers_test.go | 22 +++++++ pkg/{acsengine => helpers}/pki.go | 11 +++- pkg/{acsengine => helpers}/pki_test.go | 60 +++++++++++++++++-- 14 files changed, 217 insertions(+), 114 deletions(-) delete mode 100644 pkg/acsengine/ssh.go delete mode 100644 pkg/acsengine/ssh_test.go rename pkg/{acsengine => helpers}/filesaver.go (97%) rename pkg/{acsengine => helpers}/pki.go (95%) rename pkg/{acsengine => helpers}/pki_test.go (78%) diff --git a/cmd/dcos-upgrade.go b/cmd/dcos-upgrade.go index f0049ef43f..d8679d46b1 100644 --- a/cmd/dcos-upgrade.go +++ b/cmd/dcos-upgrade.go @@ -8,7 +8,6 @@ import ( "path" "path/filepath" - "github.com/Azure/acs-engine/pkg/acsengine" "github.com/Azure/acs-engine/pkg/api" "github.com/Azure/acs-engine/pkg/armhelpers" "github.com/Azure/acs-engine/pkg/helpers" @@ -230,7 +229,7 @@ func (uc *dcosUpgradeCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: uc.locale, }, diff --git a/cmd/deploy.go b/cmd/deploy.go index d73f760773..4ee637a592 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -294,7 +294,7 @@ func autofillApimodel(dc *deployCmd) error { translator := &i18n.Translator{ Locale: dc.locale, } - _, publicKey, err := acsengine.CreateSaveSSH(dc.containerService.Properties.LinuxProfile.AdminUsername, dc.outputDirectory, translator) + _, publicKey, err := helpers.CreateSaveSSH(dc.containerService.Properties.LinuxProfile.AdminUsername, dc.outputDirectory, translator) if err != nil { return errors.Wrap(err, "Failed to generate SSH Key") } diff --git a/cmd/scale.go b/cmd/scale.go index a00a85a3f8..5f923f0175 100644 --- a/cmd/scale.go +++ b/cmd/scale.go @@ -442,7 +442,7 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: sc.locale, }, diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 96fa1a5e35..efaf9d4e29 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -244,7 +244,7 @@ func (uc *upgradeCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: uc.locale, }, diff --git a/pkg/acsengine/defaults.go b/pkg/acsengine/defaults.go index 05ba380068..09e8bf658a 100644 --- a/pkg/acsengine/defaults.go +++ b/pkg/acsengine/defaults.go @@ -705,98 +705,98 @@ func setHostedMasterProfileDefaults(a *api.Properties) { a.HostedMasterProfile.Subnet = DefaultKubernetesMasterSubnet } -func setDefaultCerts(a *api.Properties) (bool, error) { - if a.MasterProfile != nil && a.OrchestratorProfile.OrchestratorType == api.OpenShift { - return certgen.OpenShiftSetDefaultCerts(a, api.DefaultOpenshiftOrchestratorName, a.GetClusterID()) +func setDefaultCerts(p *api.Properties) (bool, error) { + if p.MasterProfile != nil && p.OrchestratorProfile.OrchestratorType == api.OpenShift { + return certgen.OpenShiftSetDefaultCerts(p, api.DefaultOpenshiftOrchestratorName, p.GetClusterID()) } - if a.MasterProfile == nil || a.OrchestratorProfile.OrchestratorType != api.Kubernetes { + if p.MasterProfile == nil || p.OrchestratorProfile.OrchestratorType != api.Kubernetes { return false, nil } - provided := certsAlreadyPresent(a.CertificateProfile, a.MasterProfile.Count) + provided := certsAlreadyPresent(p.CertificateProfile, p.MasterProfile.Count) if areAllTrue(provided) { return false, nil } - masterExtraFQDNs := append(formatAzureProdFQDNs(a.MasterProfile.DNSPrefix), a.MasterProfile.SubjectAltNames...) - firstMasterIP := net.ParseIP(a.MasterProfile.FirstConsecutiveStaticIP).To4() + masterExtraFQDNs := append(formatAzureProdFQDNs(p.MasterProfile.DNSPrefix), p.MasterProfile.SubjectAltNames...) + firstMasterIP := net.ParseIP(p.MasterProfile.FirstConsecutiveStaticIP).To4() if firstMasterIP == nil { - return false, errors.Errorf("MasterProfile.FirstConsecutiveStaticIP '%s' is an invalid IP address", a.MasterProfile.FirstConsecutiveStaticIP) + return false, errors.Errorf("MasterProfile.FirstConsecutiveStaticIP '%s' is an invalid IP address", p.MasterProfile.FirstConsecutiveStaticIP) } ips := []net.IP{firstMasterIP} - // Add the Internal Loadbalancer IP which is always at at a known offset from the firstMasterIP + // Add the Internal Loadbalancer IP which is always at at p known offset from the firstMasterIP ips = append(ips, net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(DefaultInternalLbStaticIPOffset)}) // Include the Internal load balancer as well - if a.MasterProfile.IsVirtualMachineScaleSets() { + if p.MasterProfile.IsVirtualMachineScaleSets() { // Include the Internal load balancer as well - for i := 1; i < a.MasterProfile.Count; i++ { - offset := i * a.MasterProfile.IPAddressCount + for i := 1; i < p.MasterProfile.Count; i++ { + offset := i * p.MasterProfile.IPAddressCount ip := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(offset)} ips = append(ips, ip) } } else { - for i := 1; i < a.MasterProfile.Count; i++ { + for i := 1; i < p.MasterProfile.Count; i++ { ip := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(i)} ips = append(ips, ip) } } - if a.CertificateProfile == nil { - a.CertificateProfile = &api.CertificateProfile{} + if p.CertificateProfile == nil { + p.CertificateProfile = &api.CertificateProfile{} } - // use the specified Certificate Authority pair, or generate a new pair - var caPair *PkiKeyCertPair + // use the specified Certificate Authority pair, or generate p new pair + var caPair *helpers.PkiKeyCertPair if provided["ca"] { - caPair = &PkiKeyCertPair{CertificatePem: a.CertificateProfile.CaCertificate, PrivateKeyPem: a.CertificateProfile.CaPrivateKey} + caPair = &helpers.PkiKeyCertPair{CertificatePem: p.CertificateProfile.CaCertificate, PrivateKeyPem: p.CertificateProfile.CaPrivateKey} } else { - caCertificate, caPrivateKey, err := createCertificate("ca", nil, nil, false, false, nil, nil, nil) + var err error + caPair, err = helpers.CreatePkiKeyCertPair("ca") if err != nil { - return false, err + return false, nil } - caPair = &PkiKeyCertPair{CertificatePem: string(certificateToPem(caCertificate.Raw)), PrivateKeyPem: string(privateKeyToPem(caPrivateKey))} - a.CertificateProfile.CaCertificate = caPair.CertificatePem - a.CertificateProfile.CaPrivateKey = caPair.PrivateKeyPem + p.CertificateProfile.CaCertificate = caPair.CertificatePem + p.CertificateProfile.CaPrivateKey = caPair.PrivateKeyPem } - cidrFirstIP, err := common.CidrStringFirstIP(a.OrchestratorProfile.KubernetesConfig.ServiceCIDR) + cidrFirstIP, err := common.CidrStringFirstIP(p.OrchestratorProfile.KubernetesConfig.ServiceCIDR) if err != nil { return false, err } ips = append(ips, cidrFirstIP) - apiServerPair, clientPair, kubeConfigPair, etcdServerPair, etcdClientPair, etcdPeerPairs, err := CreatePki(masterExtraFQDNs, ips, DefaultKubernetesClusterDomain, caPair, a.MasterProfile.Count) + apiServerPair, clientPair, kubeConfigPair, etcdServerPair, etcdClientPair, etcdPeerPairs, err := helpers.CreatePki(masterExtraFQDNs, ips, DefaultKubernetesClusterDomain, caPair, p.MasterProfile.Count) if err != nil { return false, err } // If no Certificate Authority pair or no cert/key pair was provided, use generated cert/key pairs signed by provided Certificate Authority pair if !provided["apiserver"] || !provided["ca"] { - a.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem - a.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem + p.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem + p.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem } if !provided["client"] || !provided["ca"] { - a.CertificateProfile.ClientCertificate = clientPair.CertificatePem - a.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem + p.CertificateProfile.ClientCertificate = clientPair.CertificatePem + p.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem } if !provided["kubeconfig"] || !provided["ca"] { - a.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem - a.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem + p.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem + p.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem } if !provided["etcd"] || !provided["ca"] { - a.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem - a.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem - a.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem - a.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem - a.CertificateProfile.EtcdPeerCertificates = make([]string, a.MasterProfile.Count) - a.CertificateProfile.EtcdPeerPrivateKeys = make([]string, a.MasterProfile.Count) + p.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem + p.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem + p.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem + p.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem + p.CertificateProfile.EtcdPeerCertificates = make([]string, p.MasterProfile.Count) + p.CertificateProfile.EtcdPeerPrivateKeys = make([]string, p.MasterProfile.Count) for i, v := range etcdPeerPairs { - a.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem - a.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem + p.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem + p.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem } } diff --git a/pkg/acsengine/engine_test.go b/pkg/acsengine/engine_test.go index 472d186072..f577691a1f 100644 --- a/pkg/acsengine/engine_test.go +++ b/pkg/acsengine/engine_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "path" "path/filepath" + "reflect" "strings" "testing" @@ -556,3 +557,64 @@ func TestGenerateKubeConfig(t *testing.T) { t.Fatalf("Expected an error result from nil Properties child properties") } } + +func TestFormatAzureProdFQDN(t *testing.T) { + dnsPrefix := "santest" + + actual := formatAzureProdFQDNs(dnsPrefix) + + expected := []string{ + "santest.australiacentral.cloudapp.azure.com", + "santest.australiacentral2.cloudapp.azure.com", + "santest.australiaeast.cloudapp.azure.com", + "santest.australiasoutheast.cloudapp.azure.com", + "santest.brazilsouth.cloudapp.azure.com", + "santest.canadacentral.cloudapp.azure.com", + "santest.canadaeast.cloudapp.azure.com", + "santest.centralindia.cloudapp.azure.com", + "santest.centralus.cloudapp.azure.com", + "santest.centraluseuap.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.eastasia.cloudapp.azure.com", + "santest.eastus.cloudapp.azure.com", + "santest.eastus2.cloudapp.azure.com", + "santest.eastus2euap.cloudapp.azure.com", + "santest.francecentral.cloudapp.azure.com", + "santest.francesouth.cloudapp.azure.com", + "santest.japaneast.cloudapp.azure.com", + "santest.japanwest.cloudapp.azure.com", + "santest.koreacentral.cloudapp.azure.com", + "santest.koreasouth.cloudapp.azure.com", + "santest.northcentralus.cloudapp.azure.com", + "santest.northeurope.cloudapp.azure.com", + "santest.southcentralus.cloudapp.azure.com", + "santest.southeastasia.cloudapp.azure.com", + "santest.southindia.cloudapp.azure.com", + "santest.uksouth.cloudapp.azure.com", + "santest.ukwest.cloudapp.azure.com", + "santest.westcentralus.cloudapp.azure.com", + "santest.westeurope.cloudapp.azure.com", + "santest.westindia.cloudapp.azure.com", + "santest.westus.cloudapp.azure.com", + "santest.westus2.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.germanycentral.cloudapp.microsoftazure.de", + "santest.germanynortheast.cloudapp.microsoftazure.de", + "santest.usgovvirginia.cloudapp.usgovcloudapi.net", + "santest.usgoviowa.cloudapp.usgovcloudapi.net", + "santest.usgovarizona.cloudapp.usgovcloudapi.net", + "santest.usgovtexas.cloudapp.usgovcloudapi.net", + "santest.francecentral.cloudapp.azure.com", + } + + if !reflect.DeepEqual(actual, expected) { + t.Errorf("expected formatted fqdns %s, but got %s", expected, actual) + } + +} diff --git a/pkg/acsengine/output.go b/pkg/acsengine/output.go index 4a6c1b0fdd..59d237f7d7 100644 --- a/pkg/acsengine/output.go +++ b/pkg/acsengine/output.go @@ -2,6 +2,7 @@ package acsengine import ( "fmt" + "github.com/Azure/acs-engine/pkg/helpers" "io/ioutil" "path" "path/filepath" @@ -24,7 +25,7 @@ func (w *ArtifactWriter) WriteTLSArtifacts(containerService *api.ContainerServic artifactsDir = path.Join("_output", artifactsDir) } - f := &FileSaver{ + f := &helpers.FileSaver{ Translator: w.Translator, } diff --git a/pkg/acsengine/ssh.go b/pkg/acsengine/ssh.go deleted file mode 100644 index bcee867057..0000000000 --- a/pkg/acsengine/ssh.go +++ /dev/null @@ -1,32 +0,0 @@ -package acsengine - -import ( - "crypto/rand" - "crypto/rsa" - "fmt" - - "github.com/Azure/acs-engine/pkg/helpers" - "github.com/Azure/acs-engine/pkg/i18n" -) - -// CreateSaveSSH generates and stashes an SSH key pair. -func CreateSaveSSH(username, outputDirectory string, s *i18n.Translator) (privateKey *rsa.PrivateKey, publicKeyString string, err error) { - - privateKey, publicKeyString, err = helpers.CreateSSH(rand.Reader, s) - if err != nil { - return nil, "", err - } - - privateKeyPem := privateKeyToPem(privateKey) - - f := &FileSaver{ - Translator: s, - } - - err = f.SaveFile(outputDirectory, fmt.Sprintf("%s_rsa", username), privateKeyPem) - if err != nil { - return nil, "", err - } - - return privateKey, publicKeyString, nil -} diff --git a/pkg/acsengine/ssh_test.go b/pkg/acsengine/ssh_test.go deleted file mode 100644 index cb963336c5..0000000000 --- a/pkg/acsengine/ssh_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package acsengine - -import ( - "os" - "testing" - - "github.com/Azure/acs-engine/pkg/i18n" -) - -func TestCreateSaveSSH(t *testing.T) { - translator := &i18n.Translator{ - Locale: nil, - } - username := "test_user" - outputDirectory := "unit_tests" - expectedFile := outputDirectory + "/" + username + "_rsa" - - defer os.Remove(expectedFile) - - _, _, err := CreateSaveSSH(username, outputDirectory, translator) - - if err != nil { - t.Fatalf("Unexpected error creating and saving ssh key: %s", err) - } - - if _, err := os.Stat(expectedFile); os.IsNotExist(err) { - t.Fatalf("ssh file was not created") - } -} diff --git a/pkg/acsengine/filesaver.go b/pkg/helpers/filesaver.go similarity index 97% rename from pkg/acsengine/filesaver.go rename to pkg/helpers/filesaver.go index 59495750a7..a63dd9776a 100644 --- a/pkg/acsengine/filesaver.go +++ b/pkg/helpers/filesaver.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "io/ioutil" diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index cda202160c..6b96499437 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -3,8 +3,10 @@ package helpers import ( // "fmt" "bytes" + "crypto/rand" "crypto/rsa" "encoding/json" + "fmt" "io" "os" "runtime" @@ -161,3 +163,24 @@ func GetHomeDir() string { func ShellQuote(s string) string { return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'` } + +// CreateSaveSSH generates and stashes an SSH key pair. +func CreateSaveSSH(username, outputDirectory string, s *i18n.Translator) (privateKey *rsa.PrivateKey, publicKeyString string, err error) { + privateKey, publicKeyString, err = CreateSSH(rand.Reader, s) + if err != nil { + return nil, "", err + } + + privateKeyPem := privateKeyToPem(privateKey) + + f := &FileSaver{ + Translator: s, + } + + err = f.SaveFile(outputDirectory, fmt.Sprintf("%s_rsa", username), privateKeyPem) + if err != nil { + return nil, "", err + } + + return privateKey, publicKeyString, nil +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index ee16e5af89..f73117967e 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "encoding/pem" "math/rand" + "os" "os/exec" "runtime" "strings" @@ -312,3 +313,24 @@ func TestShellQuote(t *testing.T) { } } } + +func TestCreateSaveSSH(t *testing.T) { + translator := &i18n.Translator{ + Locale: nil, + } + username := "test_user" + outputDirectory := "unit_tests" + expectedFile := outputDirectory + "/" + username + "_rsa" + + defer os.Remove(expectedFile) + + _, _, err := CreateSaveSSH(username, outputDirectory, translator) + + if err != nil { + t.Fatalf("Unexpected error creating and saving ssh key: %s", err) + } + + if _, err := os.Stat(expectedFile); os.IsNotExist(err) { + t.Fatalf("ssh file was not created") + } +} diff --git a/pkg/acsengine/pki.go b/pkg/helpers/pki.go similarity index 95% rename from pkg/acsengine/pki.go rename to pkg/helpers/pki.go index a650cb4204..81aee2b9d3 100644 --- a/pkg/acsengine/pki.go +++ b/pkg/helpers/pki.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "bytes" @@ -31,6 +31,15 @@ type PkiKeyCertPair struct { PrivateKeyPem string } +func CreatePkiKeyCertPair(commonName string) (*PkiKeyCertPair, error) { + caCertificate, caPrivateKey, err := createCertificate("ca", nil, nil, false, false, nil, nil, nil) + if err != nil { + return nil, err + } + caPair := &PkiKeyCertPair{CertificatePem: string(certificateToPem(caCertificate.Raw)), PrivateKeyPem: string(privateKeyToPem(caPrivateKey))} + return caPair, nil +} + // CreatePki creates PKI certificates func CreatePki(extraFQDNs []string, extraIPs []net.IP, clusterDomain string, caPair *PkiKeyCertPair, masterCount int) (*PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, []*PkiKeyCertPair, error) { start := time.Now() diff --git a/pkg/acsengine/pki_test.go b/pkg/helpers/pki_test.go similarity index 78% rename from pkg/acsengine/pki_test.go rename to pkg/helpers/pki_test.go index accd6b615e..c048e4b18d 100644 --- a/pkg/acsengine/pki_test.go +++ b/pkg/helpers/pki_test.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "crypto/rsa" @@ -85,7 +85,6 @@ func TestCreateCertificateWithoutOrganisation(t *testing.T) { } func TestSubjectAltNameInCert(t *testing.T) { - dnsPrefix := "santest" extraIPs := []net.IP{net.ParseIP("10.255.255.5"), net.ParseIP("10.255.255.15")} roots := x509.NewCertPool() @@ -103,10 +102,59 @@ func TestSubjectAltNameInCert(t *testing.T) { // Test SAN is not empty SubjectAltNames := []string{"santest.mydomain.com", "santest2.testdomain.net"} - masterExtraFQDNs := append(formatAzureProdFQDNs(dnsPrefix), SubjectAltNames...) + formattedDNSPrefixes := []string{ + "santest.australiacentral.cloudapp.azure.com", + "santest.australiacentral2.cloudapp.azure.com", + "santest.australiaeast.cloudapp.azure.com", + "santest.australiasoutheast.cloudapp.azure.com", + "santest.brazilsouth.cloudapp.azure.com", + "santest.canadacentral.cloudapp.azure.com", + "santest.canadaeast.cloudapp.azure.com", + "santest.centralindia.cloudapp.azure.com", + "santest.centralus.cloudapp.azure.com", + "santest.centraluseuap.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.eastasia.cloudapp.azure.com", + "santest.eastus.cloudapp.azure.com", + "santest.eastus2.cloudapp.azure.com", + "santest.eastus2euap.cloudapp.azure.com", + "santest.francecentral.cloudapp.azure.com", + "santest.francesouth.cloudapp.azure.com", + "santest.japaneast.cloudapp.azure.com", + "santest.japanwest.cloudapp.azure.com", + "santest.koreacentral.cloudapp.azure.com", + "santest.koreasouth.cloudapp.azure.com", + "santest.northcentralus.cloudapp.azure.com", + "santest.northeurope.cloudapp.azure.com", + "santest.southcentralus.cloudapp.azure.com", + "santest.southeastasia.cloudapp.azure.com", + "santest.southindia.cloudapp.azure.com", + "santest.uksouth.cloudapp.azure.com", + "santest.ukwest.cloudapp.azure.com", + "santest.westcentralus.cloudapp.azure.com", + "santest.westeurope.cloudapp.azure.com", + "santest.westindia.cloudapp.azure.com", + "santest.westus.cloudapp.azure.com", + "santest.westus2.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.germanycentral.cloudapp.microsoftazure.de", + "santest.germanynortheast.cloudapp.microsoftazure.de", + "santest.usgovvirginia.cloudapp.usgovcloudapi.net", + "santest.usgoviowa.cloudapp.usgovcloudapi.net", + "santest.usgovarizona.cloudapp.usgovcloudapi.net", + "santest.usgovtexas.cloudapp.usgovcloudapi.net", + "santest.francecentral.cloudapp.azure.com", + } + masterExtraFQDNs := append(formattedDNSPrefixes, SubjectAltNames...) expectedDNSNames := []string{"santest.australiaeast.cloudapp.azure.com", "santest.australiasoutheast.cloudapp.azure.com", "santest.brazilsouth.cloudapp.azure.com", "santest.canadacentral.cloudapp.azure.com", "santest.canadaeast.cloudapp.azure.com", "santest.centralindia.cloudapp.azure.com", "santest.centralus.cloudapp.azure.com", "santest.centraluseuap.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinaeast2.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.chinanorth2.cloudapp.chinacloudapi.cn", "santest.eastasia.cloudapp.azure.com", "santest.eastus.cloudapp.azure.com", "santest.eastus2.cloudapp.azure.com", "santest.eastus2euap.cloudapp.azure.com", "santest.japaneast.cloudapp.azure.com", "santest.japanwest.cloudapp.azure.com", "santest.koreacentral.cloudapp.azure.com", "santest.koreasouth.cloudapp.azure.com", "santest.northcentralus.cloudapp.azure.com", "santest.northeurope.cloudapp.azure.com", "santest.southcentralus.cloudapp.azure.com", "santest.southeastasia.cloudapp.azure.com", "santest.southindia.cloudapp.azure.com", "santest.uksouth.cloudapp.azure.com", "santest.ukwest.cloudapp.azure.com", "santest.westcentralus.cloudapp.azure.com", "santest.westeurope.cloudapp.azure.com", "santest.westindia.cloudapp.azure.com", "santest.westus.cloudapp.azure.com", "santest.westus2.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.germanycentral.cloudapp.microsoftazure.de", "santest.germanynortheast.cloudapp.microsoftazure.de", "santest.usgovvirginia.cloudapp.usgovcloudapi.net", "santest.usgoviowa.cloudapp.usgovcloudapi.net", "santest.usgovarizona.cloudapp.usgovcloudapi.net", "santest.usgovtexas.cloudapp.usgovcloudapi.net", "santest.francecentral.cloudapp.azure.com", "santest.mydomain.com", "santest2.testdomain.net", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", "kubernetes.kube-system", "kubernetes.kube-system.svc", "kubernetes.kube-system.svc.cluster.local"} - apiServerPair, _, _, _, _, _, err := CreatePki(masterExtraFQDNs, extraIPs, DefaultKubernetesClusterDomain, caPair, 1) + apiServerPair, _, _, _, _, _, err := CreatePki(masterExtraFQDNs, extraIPs, "cluster.local", caPair, 1) if err != nil { t.Fatalf("failed to generate certificates: %s.", err) @@ -139,10 +187,10 @@ func TestSubjectAltNameInCert(t *testing.T) { // Test SAN is empty SubjectAltNames = []string{} - masterExtraFQDNs = append(formatAzureProdFQDNs(dnsPrefix), SubjectAltNames...) + masterExtraFQDNs = append(formattedDNSPrefixes, SubjectAltNames...) expectedDNSNames = []string{"santest.australiaeast.cloudapp.azure.com", "santest.australiasoutheast.cloudapp.azure.com", "santest.brazilsouth.cloudapp.azure.com", "santest.canadacentral.cloudapp.azure.com", "santest.canadaeast.cloudapp.azure.com", "santest.centralindia.cloudapp.azure.com", "santest.centralus.cloudapp.azure.com", "santest.centraluseuap.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinaeast2.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.chinanorth2.cloudapp.chinacloudapi.cn", "santest.eastasia.cloudapp.azure.com", "santest.eastus.cloudapp.azure.com", "santest.eastus2.cloudapp.azure.com", "santest.eastus2euap.cloudapp.azure.com", "santest.japaneast.cloudapp.azure.com", "santest.japanwest.cloudapp.azure.com", "santest.koreacentral.cloudapp.azure.com", "santest.koreasouth.cloudapp.azure.com", "santest.northcentralus.cloudapp.azure.com", "santest.northeurope.cloudapp.azure.com", "santest.southcentralus.cloudapp.azure.com", "santest.southeastasia.cloudapp.azure.com", "santest.southindia.cloudapp.azure.com", "santest.uksouth.cloudapp.azure.com", "santest.ukwest.cloudapp.azure.com", "santest.westcentralus.cloudapp.azure.com", "santest.westeurope.cloudapp.azure.com", "santest.westindia.cloudapp.azure.com", "santest.westus.cloudapp.azure.com", "santest.westus2.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.germanycentral.cloudapp.microsoftazure.de", "santest.germanynortheast.cloudapp.microsoftazure.de", "santest.usgovvirginia.cloudapp.usgovcloudapi.net", "santest.usgoviowa.cloudapp.usgovcloudapi.net", "santest.usgovarizona.cloudapp.usgovcloudapi.net", "santest.usgovtexas.cloudapp.usgovcloudapi.net", "santest.francecentral.cloudapp.azure.com", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", "kubernetes.kube-system", "kubernetes.kube-system.svc", "kubernetes.kube-system.svc.cluster.local"} - apiServerPair, _, _, _, _, _, err = CreatePki(masterExtraFQDNs, extraIPs, DefaultKubernetesClusterDomain, caPair, 1) + apiServerPair, _, _, _, _, _, err = CreatePki(masterExtraFQDNs, extraIPs, "cluster.local", caPair, 1) if err != nil { t.Fatalf("failed to generate certificates: %s.", err) From ed7b0d0ab14270cf5186f15622b12ec26ad160b4 Mon Sep 17 00:00:00 2001 From: tariqibrahim Date: Wed, 3 Oct 2018 17:19:17 -0600 Subject: [PATCH 2/4] Move utility methods to the helper package --- cmd/dcos-upgrade.go | 3 +- cmd/deploy.go | 2 +- cmd/scale.go | 2 +- cmd/upgrade.go | 2 +- pkg/acsengine/defaults.go | 78 ++++++++++++------------- pkg/acsengine/engine_test.go | 62 ++++++++++++++++++++ pkg/acsengine/output.go | 3 +- pkg/acsengine/ssh.go | 32 ---------- pkg/acsengine/ssh_test.go | 29 --------- pkg/{acsengine => helpers}/filesaver.go | 2 +- pkg/helpers/helpers.go | 23 ++++++++ pkg/helpers/helpers_test.go | 22 +++++++ pkg/{acsengine => helpers}/pki.go | 12 +++- pkg/{acsengine => helpers}/pki_test.go | 60 +++++++++++++++++-- 14 files changed, 218 insertions(+), 114 deletions(-) delete mode 100644 pkg/acsengine/ssh.go delete mode 100644 pkg/acsengine/ssh_test.go rename pkg/{acsengine => helpers}/filesaver.go (97%) rename pkg/{acsengine => helpers}/pki.go (94%) rename pkg/{acsengine => helpers}/pki_test.go (78%) diff --git a/cmd/dcos-upgrade.go b/cmd/dcos-upgrade.go index f0049ef43f..d8679d46b1 100644 --- a/cmd/dcos-upgrade.go +++ b/cmd/dcos-upgrade.go @@ -8,7 +8,6 @@ import ( "path" "path/filepath" - "github.com/Azure/acs-engine/pkg/acsengine" "github.com/Azure/acs-engine/pkg/api" "github.com/Azure/acs-engine/pkg/armhelpers" "github.com/Azure/acs-engine/pkg/helpers" @@ -230,7 +229,7 @@ func (uc *dcosUpgradeCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: uc.locale, }, diff --git a/cmd/deploy.go b/cmd/deploy.go index d73f760773..4ee637a592 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -294,7 +294,7 @@ func autofillApimodel(dc *deployCmd) error { translator := &i18n.Translator{ Locale: dc.locale, } - _, publicKey, err := acsengine.CreateSaveSSH(dc.containerService.Properties.LinuxProfile.AdminUsername, dc.outputDirectory, translator) + _, publicKey, err := helpers.CreateSaveSSH(dc.containerService.Properties.LinuxProfile.AdminUsername, dc.outputDirectory, translator) if err != nil { return errors.Wrap(err, "Failed to generate SSH Key") } diff --git a/cmd/scale.go b/cmd/scale.go index a00a85a3f8..5f923f0175 100644 --- a/cmd/scale.go +++ b/cmd/scale.go @@ -442,7 +442,7 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: sc.locale, }, diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 96fa1a5e35..efaf9d4e29 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -244,7 +244,7 @@ func (uc *upgradeCmd) run(cmd *cobra.Command, args []string) error { return err } - f := acsengine.FileSaver{ + f := helpers.FileSaver{ Translator: &i18n.Translator{ Locale: uc.locale, }, diff --git a/pkg/acsengine/defaults.go b/pkg/acsengine/defaults.go index 05ba380068..09e8bf658a 100644 --- a/pkg/acsengine/defaults.go +++ b/pkg/acsengine/defaults.go @@ -705,98 +705,98 @@ func setHostedMasterProfileDefaults(a *api.Properties) { a.HostedMasterProfile.Subnet = DefaultKubernetesMasterSubnet } -func setDefaultCerts(a *api.Properties) (bool, error) { - if a.MasterProfile != nil && a.OrchestratorProfile.OrchestratorType == api.OpenShift { - return certgen.OpenShiftSetDefaultCerts(a, api.DefaultOpenshiftOrchestratorName, a.GetClusterID()) +func setDefaultCerts(p *api.Properties) (bool, error) { + if p.MasterProfile != nil && p.OrchestratorProfile.OrchestratorType == api.OpenShift { + return certgen.OpenShiftSetDefaultCerts(p, api.DefaultOpenshiftOrchestratorName, p.GetClusterID()) } - if a.MasterProfile == nil || a.OrchestratorProfile.OrchestratorType != api.Kubernetes { + if p.MasterProfile == nil || p.OrchestratorProfile.OrchestratorType != api.Kubernetes { return false, nil } - provided := certsAlreadyPresent(a.CertificateProfile, a.MasterProfile.Count) + provided := certsAlreadyPresent(p.CertificateProfile, p.MasterProfile.Count) if areAllTrue(provided) { return false, nil } - masterExtraFQDNs := append(formatAzureProdFQDNs(a.MasterProfile.DNSPrefix), a.MasterProfile.SubjectAltNames...) - firstMasterIP := net.ParseIP(a.MasterProfile.FirstConsecutiveStaticIP).To4() + masterExtraFQDNs := append(formatAzureProdFQDNs(p.MasterProfile.DNSPrefix), p.MasterProfile.SubjectAltNames...) + firstMasterIP := net.ParseIP(p.MasterProfile.FirstConsecutiveStaticIP).To4() if firstMasterIP == nil { - return false, errors.Errorf("MasterProfile.FirstConsecutiveStaticIP '%s' is an invalid IP address", a.MasterProfile.FirstConsecutiveStaticIP) + return false, errors.Errorf("MasterProfile.FirstConsecutiveStaticIP '%s' is an invalid IP address", p.MasterProfile.FirstConsecutiveStaticIP) } ips := []net.IP{firstMasterIP} - // Add the Internal Loadbalancer IP which is always at at a known offset from the firstMasterIP + // Add the Internal Loadbalancer IP which is always at at p known offset from the firstMasterIP ips = append(ips, net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(DefaultInternalLbStaticIPOffset)}) // Include the Internal load balancer as well - if a.MasterProfile.IsVirtualMachineScaleSets() { + if p.MasterProfile.IsVirtualMachineScaleSets() { // Include the Internal load balancer as well - for i := 1; i < a.MasterProfile.Count; i++ { - offset := i * a.MasterProfile.IPAddressCount + for i := 1; i < p.MasterProfile.Count; i++ { + offset := i * p.MasterProfile.IPAddressCount ip := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(offset)} ips = append(ips, ip) } } else { - for i := 1; i < a.MasterProfile.Count; i++ { + for i := 1; i < p.MasterProfile.Count; i++ { ip := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(i)} ips = append(ips, ip) } } - if a.CertificateProfile == nil { - a.CertificateProfile = &api.CertificateProfile{} + if p.CertificateProfile == nil { + p.CertificateProfile = &api.CertificateProfile{} } - // use the specified Certificate Authority pair, or generate a new pair - var caPair *PkiKeyCertPair + // use the specified Certificate Authority pair, or generate p new pair + var caPair *helpers.PkiKeyCertPair if provided["ca"] { - caPair = &PkiKeyCertPair{CertificatePem: a.CertificateProfile.CaCertificate, PrivateKeyPem: a.CertificateProfile.CaPrivateKey} + caPair = &helpers.PkiKeyCertPair{CertificatePem: p.CertificateProfile.CaCertificate, PrivateKeyPem: p.CertificateProfile.CaPrivateKey} } else { - caCertificate, caPrivateKey, err := createCertificate("ca", nil, nil, false, false, nil, nil, nil) + var err error + caPair, err = helpers.CreatePkiKeyCertPair("ca") if err != nil { - return false, err + return false, nil } - caPair = &PkiKeyCertPair{CertificatePem: string(certificateToPem(caCertificate.Raw)), PrivateKeyPem: string(privateKeyToPem(caPrivateKey))} - a.CertificateProfile.CaCertificate = caPair.CertificatePem - a.CertificateProfile.CaPrivateKey = caPair.PrivateKeyPem + p.CertificateProfile.CaCertificate = caPair.CertificatePem + p.CertificateProfile.CaPrivateKey = caPair.PrivateKeyPem } - cidrFirstIP, err := common.CidrStringFirstIP(a.OrchestratorProfile.KubernetesConfig.ServiceCIDR) + cidrFirstIP, err := common.CidrStringFirstIP(p.OrchestratorProfile.KubernetesConfig.ServiceCIDR) if err != nil { return false, err } ips = append(ips, cidrFirstIP) - apiServerPair, clientPair, kubeConfigPair, etcdServerPair, etcdClientPair, etcdPeerPairs, err := CreatePki(masterExtraFQDNs, ips, DefaultKubernetesClusterDomain, caPair, a.MasterProfile.Count) + apiServerPair, clientPair, kubeConfigPair, etcdServerPair, etcdClientPair, etcdPeerPairs, err := helpers.CreatePki(masterExtraFQDNs, ips, DefaultKubernetesClusterDomain, caPair, p.MasterProfile.Count) if err != nil { return false, err } // If no Certificate Authority pair or no cert/key pair was provided, use generated cert/key pairs signed by provided Certificate Authority pair if !provided["apiserver"] || !provided["ca"] { - a.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem - a.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem + p.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem + p.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem } if !provided["client"] || !provided["ca"] { - a.CertificateProfile.ClientCertificate = clientPair.CertificatePem - a.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem + p.CertificateProfile.ClientCertificate = clientPair.CertificatePem + p.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem } if !provided["kubeconfig"] || !provided["ca"] { - a.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem - a.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem + p.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem + p.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem } if !provided["etcd"] || !provided["ca"] { - a.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem - a.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem - a.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem - a.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem - a.CertificateProfile.EtcdPeerCertificates = make([]string, a.MasterProfile.Count) - a.CertificateProfile.EtcdPeerPrivateKeys = make([]string, a.MasterProfile.Count) + p.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem + p.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem + p.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem + p.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem + p.CertificateProfile.EtcdPeerCertificates = make([]string, p.MasterProfile.Count) + p.CertificateProfile.EtcdPeerPrivateKeys = make([]string, p.MasterProfile.Count) for i, v := range etcdPeerPairs { - a.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem - a.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem + p.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem + p.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem } } diff --git a/pkg/acsengine/engine_test.go b/pkg/acsengine/engine_test.go index 472d186072..f577691a1f 100644 --- a/pkg/acsengine/engine_test.go +++ b/pkg/acsengine/engine_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "path" "path/filepath" + "reflect" "strings" "testing" @@ -556,3 +557,64 @@ func TestGenerateKubeConfig(t *testing.T) { t.Fatalf("Expected an error result from nil Properties child properties") } } + +func TestFormatAzureProdFQDN(t *testing.T) { + dnsPrefix := "santest" + + actual := formatAzureProdFQDNs(dnsPrefix) + + expected := []string{ + "santest.australiacentral.cloudapp.azure.com", + "santest.australiacentral2.cloudapp.azure.com", + "santest.australiaeast.cloudapp.azure.com", + "santest.australiasoutheast.cloudapp.azure.com", + "santest.brazilsouth.cloudapp.azure.com", + "santest.canadacentral.cloudapp.azure.com", + "santest.canadaeast.cloudapp.azure.com", + "santest.centralindia.cloudapp.azure.com", + "santest.centralus.cloudapp.azure.com", + "santest.centraluseuap.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.eastasia.cloudapp.azure.com", + "santest.eastus.cloudapp.azure.com", + "santest.eastus2.cloudapp.azure.com", + "santest.eastus2euap.cloudapp.azure.com", + "santest.francecentral.cloudapp.azure.com", + "santest.francesouth.cloudapp.azure.com", + "santest.japaneast.cloudapp.azure.com", + "santest.japanwest.cloudapp.azure.com", + "santest.koreacentral.cloudapp.azure.com", + "santest.koreasouth.cloudapp.azure.com", + "santest.northcentralus.cloudapp.azure.com", + "santest.northeurope.cloudapp.azure.com", + "santest.southcentralus.cloudapp.azure.com", + "santest.southeastasia.cloudapp.azure.com", + "santest.southindia.cloudapp.azure.com", + "santest.uksouth.cloudapp.azure.com", + "santest.ukwest.cloudapp.azure.com", + "santest.westcentralus.cloudapp.azure.com", + "santest.westeurope.cloudapp.azure.com", + "santest.westindia.cloudapp.azure.com", + "santest.westus.cloudapp.azure.com", + "santest.westus2.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.germanycentral.cloudapp.microsoftazure.de", + "santest.germanynortheast.cloudapp.microsoftazure.de", + "santest.usgovvirginia.cloudapp.usgovcloudapi.net", + "santest.usgoviowa.cloudapp.usgovcloudapi.net", + "santest.usgovarizona.cloudapp.usgovcloudapi.net", + "santest.usgovtexas.cloudapp.usgovcloudapi.net", + "santest.francecentral.cloudapp.azure.com", + } + + if !reflect.DeepEqual(actual, expected) { + t.Errorf("expected formatted fqdns %s, but got %s", expected, actual) + } + +} diff --git a/pkg/acsengine/output.go b/pkg/acsengine/output.go index 4a6c1b0fdd..67da3eb065 100644 --- a/pkg/acsengine/output.go +++ b/pkg/acsengine/output.go @@ -8,6 +8,7 @@ import ( "strconv" "github.com/Azure/acs-engine/pkg/api" + "github.com/Azure/acs-engine/pkg/helpers" "github.com/Azure/acs-engine/pkg/i18n" "github.com/pkg/errors" ) @@ -24,7 +25,7 @@ func (w *ArtifactWriter) WriteTLSArtifacts(containerService *api.ContainerServic artifactsDir = path.Join("_output", artifactsDir) } - f := &FileSaver{ + f := &helpers.FileSaver{ Translator: w.Translator, } diff --git a/pkg/acsengine/ssh.go b/pkg/acsengine/ssh.go deleted file mode 100644 index bcee867057..0000000000 --- a/pkg/acsengine/ssh.go +++ /dev/null @@ -1,32 +0,0 @@ -package acsengine - -import ( - "crypto/rand" - "crypto/rsa" - "fmt" - - "github.com/Azure/acs-engine/pkg/helpers" - "github.com/Azure/acs-engine/pkg/i18n" -) - -// CreateSaveSSH generates and stashes an SSH key pair. -func CreateSaveSSH(username, outputDirectory string, s *i18n.Translator) (privateKey *rsa.PrivateKey, publicKeyString string, err error) { - - privateKey, publicKeyString, err = helpers.CreateSSH(rand.Reader, s) - if err != nil { - return nil, "", err - } - - privateKeyPem := privateKeyToPem(privateKey) - - f := &FileSaver{ - Translator: s, - } - - err = f.SaveFile(outputDirectory, fmt.Sprintf("%s_rsa", username), privateKeyPem) - if err != nil { - return nil, "", err - } - - return privateKey, publicKeyString, nil -} diff --git a/pkg/acsengine/ssh_test.go b/pkg/acsengine/ssh_test.go deleted file mode 100644 index cb963336c5..0000000000 --- a/pkg/acsengine/ssh_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package acsengine - -import ( - "os" - "testing" - - "github.com/Azure/acs-engine/pkg/i18n" -) - -func TestCreateSaveSSH(t *testing.T) { - translator := &i18n.Translator{ - Locale: nil, - } - username := "test_user" - outputDirectory := "unit_tests" - expectedFile := outputDirectory + "/" + username + "_rsa" - - defer os.Remove(expectedFile) - - _, _, err := CreateSaveSSH(username, outputDirectory, translator) - - if err != nil { - t.Fatalf("Unexpected error creating and saving ssh key: %s", err) - } - - if _, err := os.Stat(expectedFile); os.IsNotExist(err) { - t.Fatalf("ssh file was not created") - } -} diff --git a/pkg/acsengine/filesaver.go b/pkg/helpers/filesaver.go similarity index 97% rename from pkg/acsengine/filesaver.go rename to pkg/helpers/filesaver.go index 59495750a7..a63dd9776a 100644 --- a/pkg/acsengine/filesaver.go +++ b/pkg/helpers/filesaver.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "io/ioutil" diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index cda202160c..6b96499437 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -3,8 +3,10 @@ package helpers import ( // "fmt" "bytes" + "crypto/rand" "crypto/rsa" "encoding/json" + "fmt" "io" "os" "runtime" @@ -161,3 +163,24 @@ func GetHomeDir() string { func ShellQuote(s string) string { return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'` } + +// CreateSaveSSH generates and stashes an SSH key pair. +func CreateSaveSSH(username, outputDirectory string, s *i18n.Translator) (privateKey *rsa.PrivateKey, publicKeyString string, err error) { + privateKey, publicKeyString, err = CreateSSH(rand.Reader, s) + if err != nil { + return nil, "", err + } + + privateKeyPem := privateKeyToPem(privateKey) + + f := &FileSaver{ + Translator: s, + } + + err = f.SaveFile(outputDirectory, fmt.Sprintf("%s_rsa", username), privateKeyPem) + if err != nil { + return nil, "", err + } + + return privateKey, publicKeyString, nil +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index ee16e5af89..f73117967e 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "encoding/pem" "math/rand" + "os" "os/exec" "runtime" "strings" @@ -312,3 +313,24 @@ func TestShellQuote(t *testing.T) { } } } + +func TestCreateSaveSSH(t *testing.T) { + translator := &i18n.Translator{ + Locale: nil, + } + username := "test_user" + outputDirectory := "unit_tests" + expectedFile := outputDirectory + "/" + username + "_rsa" + + defer os.Remove(expectedFile) + + _, _, err := CreateSaveSSH(username, outputDirectory, translator) + + if err != nil { + t.Fatalf("Unexpected error creating and saving ssh key: %s", err) + } + + if _, err := os.Stat(expectedFile); os.IsNotExist(err) { + t.Fatalf("ssh file was not created") + } +} diff --git a/pkg/acsengine/pki.go b/pkg/helpers/pki.go similarity index 94% rename from pkg/acsengine/pki.go rename to pkg/helpers/pki.go index a650cb4204..6cb219d9ae 100644 --- a/pkg/acsengine/pki.go +++ b/pkg/helpers/pki.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "bytes" @@ -31,6 +31,16 @@ type PkiKeyCertPair struct { PrivateKeyPem string } +// CreatePkiKeyCertPair generates a pair of PKI certificate and private key +func CreatePkiKeyCertPair(commonName string) (*PkiKeyCertPair, error) { + caCertificate, caPrivateKey, err := createCertificate(commonName, nil, nil, false, false, nil, nil, nil) + if err != nil { + return nil, err + } + caPair := &PkiKeyCertPair{CertificatePem: string(certificateToPem(caCertificate.Raw)), PrivateKeyPem: string(privateKeyToPem(caPrivateKey))} + return caPair, nil +} + // CreatePki creates PKI certificates func CreatePki(extraFQDNs []string, extraIPs []net.IP, clusterDomain string, caPair *PkiKeyCertPair, masterCount int) (*PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, *PkiKeyCertPair, []*PkiKeyCertPair, error) { start := time.Now() diff --git a/pkg/acsengine/pki_test.go b/pkg/helpers/pki_test.go similarity index 78% rename from pkg/acsengine/pki_test.go rename to pkg/helpers/pki_test.go index accd6b615e..c048e4b18d 100644 --- a/pkg/acsengine/pki_test.go +++ b/pkg/helpers/pki_test.go @@ -1,4 +1,4 @@ -package acsengine +package helpers import ( "crypto/rsa" @@ -85,7 +85,6 @@ func TestCreateCertificateWithoutOrganisation(t *testing.T) { } func TestSubjectAltNameInCert(t *testing.T) { - dnsPrefix := "santest" extraIPs := []net.IP{net.ParseIP("10.255.255.5"), net.ParseIP("10.255.255.15")} roots := x509.NewCertPool() @@ -103,10 +102,59 @@ func TestSubjectAltNameInCert(t *testing.T) { // Test SAN is not empty SubjectAltNames := []string{"santest.mydomain.com", "santest2.testdomain.net"} - masterExtraFQDNs := append(formatAzureProdFQDNs(dnsPrefix), SubjectAltNames...) + formattedDNSPrefixes := []string{ + "santest.australiacentral.cloudapp.azure.com", + "santest.australiacentral2.cloudapp.azure.com", + "santest.australiaeast.cloudapp.azure.com", + "santest.australiasoutheast.cloudapp.azure.com", + "santest.brazilsouth.cloudapp.azure.com", + "santest.canadacentral.cloudapp.azure.com", + "santest.canadaeast.cloudapp.azure.com", + "santest.centralindia.cloudapp.azure.com", + "santest.centralus.cloudapp.azure.com", + "santest.centraluseuap.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.eastasia.cloudapp.azure.com", + "santest.eastus.cloudapp.azure.com", + "santest.eastus2.cloudapp.azure.com", + "santest.eastus2euap.cloudapp.azure.com", + "santest.francecentral.cloudapp.azure.com", + "santest.francesouth.cloudapp.azure.com", + "santest.japaneast.cloudapp.azure.com", + "santest.japanwest.cloudapp.azure.com", + "santest.koreacentral.cloudapp.azure.com", + "santest.koreasouth.cloudapp.azure.com", + "santest.northcentralus.cloudapp.azure.com", + "santest.northeurope.cloudapp.azure.com", + "santest.southcentralus.cloudapp.azure.com", + "santest.southeastasia.cloudapp.azure.com", + "santest.southindia.cloudapp.azure.com", + "santest.uksouth.cloudapp.azure.com", + "santest.ukwest.cloudapp.azure.com", + "santest.westcentralus.cloudapp.azure.com", + "santest.westeurope.cloudapp.azure.com", + "santest.westindia.cloudapp.azure.com", + "santest.westus.cloudapp.azure.com", + "santest.westus2.cloudapp.azure.com", + "santest.chinaeast.cloudapp.chinacloudapi.cn", + "santest.chinanorth.cloudapp.chinacloudapi.cn", + "santest.chinanorth2.cloudapp.chinacloudapi.cn", + "santest.chinaeast2.cloudapp.chinacloudapi.cn", + "santest.germanycentral.cloudapp.microsoftazure.de", + "santest.germanynortheast.cloudapp.microsoftazure.de", + "santest.usgovvirginia.cloudapp.usgovcloudapi.net", + "santest.usgoviowa.cloudapp.usgovcloudapi.net", + "santest.usgovarizona.cloudapp.usgovcloudapi.net", + "santest.usgovtexas.cloudapp.usgovcloudapi.net", + "santest.francecentral.cloudapp.azure.com", + } + masterExtraFQDNs := append(formattedDNSPrefixes, SubjectAltNames...) expectedDNSNames := []string{"santest.australiaeast.cloudapp.azure.com", "santest.australiasoutheast.cloudapp.azure.com", "santest.brazilsouth.cloudapp.azure.com", "santest.canadacentral.cloudapp.azure.com", "santest.canadaeast.cloudapp.azure.com", "santest.centralindia.cloudapp.azure.com", "santest.centralus.cloudapp.azure.com", "santest.centraluseuap.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinaeast2.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.chinanorth2.cloudapp.chinacloudapi.cn", "santest.eastasia.cloudapp.azure.com", "santest.eastus.cloudapp.azure.com", "santest.eastus2.cloudapp.azure.com", "santest.eastus2euap.cloudapp.azure.com", "santest.japaneast.cloudapp.azure.com", "santest.japanwest.cloudapp.azure.com", "santest.koreacentral.cloudapp.azure.com", "santest.koreasouth.cloudapp.azure.com", "santest.northcentralus.cloudapp.azure.com", "santest.northeurope.cloudapp.azure.com", "santest.southcentralus.cloudapp.azure.com", "santest.southeastasia.cloudapp.azure.com", "santest.southindia.cloudapp.azure.com", "santest.uksouth.cloudapp.azure.com", "santest.ukwest.cloudapp.azure.com", "santest.westcentralus.cloudapp.azure.com", "santest.westeurope.cloudapp.azure.com", "santest.westindia.cloudapp.azure.com", "santest.westus.cloudapp.azure.com", "santest.westus2.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.germanycentral.cloudapp.microsoftazure.de", "santest.germanynortheast.cloudapp.microsoftazure.de", "santest.usgovvirginia.cloudapp.usgovcloudapi.net", "santest.usgoviowa.cloudapp.usgovcloudapi.net", "santest.usgovarizona.cloudapp.usgovcloudapi.net", "santest.usgovtexas.cloudapp.usgovcloudapi.net", "santest.francecentral.cloudapp.azure.com", "santest.mydomain.com", "santest2.testdomain.net", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", "kubernetes.kube-system", "kubernetes.kube-system.svc", "kubernetes.kube-system.svc.cluster.local"} - apiServerPair, _, _, _, _, _, err := CreatePki(masterExtraFQDNs, extraIPs, DefaultKubernetesClusterDomain, caPair, 1) + apiServerPair, _, _, _, _, _, err := CreatePki(masterExtraFQDNs, extraIPs, "cluster.local", caPair, 1) if err != nil { t.Fatalf("failed to generate certificates: %s.", err) @@ -139,10 +187,10 @@ func TestSubjectAltNameInCert(t *testing.T) { // Test SAN is empty SubjectAltNames = []string{} - masterExtraFQDNs = append(formatAzureProdFQDNs(dnsPrefix), SubjectAltNames...) + masterExtraFQDNs = append(formattedDNSPrefixes, SubjectAltNames...) expectedDNSNames = []string{"santest.australiaeast.cloudapp.azure.com", "santest.australiasoutheast.cloudapp.azure.com", "santest.brazilsouth.cloudapp.azure.com", "santest.canadacentral.cloudapp.azure.com", "santest.canadaeast.cloudapp.azure.com", "santest.centralindia.cloudapp.azure.com", "santest.centralus.cloudapp.azure.com", "santest.centraluseuap.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinaeast2.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.chinanorth2.cloudapp.chinacloudapi.cn", "santest.eastasia.cloudapp.azure.com", "santest.eastus.cloudapp.azure.com", "santest.eastus2.cloudapp.azure.com", "santest.eastus2euap.cloudapp.azure.com", "santest.japaneast.cloudapp.azure.com", "santest.japanwest.cloudapp.azure.com", "santest.koreacentral.cloudapp.azure.com", "santest.koreasouth.cloudapp.azure.com", "santest.northcentralus.cloudapp.azure.com", "santest.northeurope.cloudapp.azure.com", "santest.southcentralus.cloudapp.azure.com", "santest.southeastasia.cloudapp.azure.com", "santest.southindia.cloudapp.azure.com", "santest.uksouth.cloudapp.azure.com", "santest.ukwest.cloudapp.azure.com", "santest.westcentralus.cloudapp.azure.com", "santest.westeurope.cloudapp.azure.com", "santest.westindia.cloudapp.azure.com", "santest.westus.cloudapp.azure.com", "santest.westus2.cloudapp.azure.com", "santest.chinaeast.cloudapp.chinacloudapi.cn", "santest.chinanorth.cloudapp.chinacloudapi.cn", "santest.germanycentral.cloudapp.microsoftazure.de", "santest.germanynortheast.cloudapp.microsoftazure.de", "santest.usgovvirginia.cloudapp.usgovcloudapi.net", "santest.usgoviowa.cloudapp.usgovcloudapi.net", "santest.usgovarizona.cloudapp.usgovcloudapi.net", "santest.usgovtexas.cloudapp.usgovcloudapi.net", "santest.francecentral.cloudapp.azure.com", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", "kubernetes.kube-system", "kubernetes.kube-system.svc", "kubernetes.kube-system.svc.cluster.local"} - apiServerPair, _, _, _, _, _, err = CreatePki(masterExtraFQDNs, extraIPs, DefaultKubernetesClusterDomain, caPair, 1) + apiServerPair, _, _, _, _, _, err = CreatePki(masterExtraFQDNs, extraIPs, "cluster.local", caPair, 1) if err != nil { t.Fatalf("failed to generate certificates: %s.", err) From afdbebabd7aa734e6048e30939c5ca982919fe1e Mon Sep 17 00:00:00 2001 From: tariqibrahim Date: Wed, 3 Oct 2018 17:38:17 -0600 Subject: [PATCH 3/4] Move utility methods to the helper package --- pkg/acsengine/output.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/acsengine/output.go b/pkg/acsengine/output.go index 0916ef352b..67da3eb065 100644 --- a/pkg/acsengine/output.go +++ b/pkg/acsengine/output.go @@ -2,7 +2,6 @@ package acsengine import ( "fmt" - "github.com/Azure/acs-engine/pkg/helpers" "io/ioutil" "path" "path/filepath" From 786afc3977ae340aa96bdcd3dea3097616a8092e Mon Sep 17 00:00:00 2001 From: tariqibrahim Date: Wed, 3 Oct 2018 17:38:17 -0600 Subject: [PATCH 4/4] Move utility methods to the helper package --- pkg/acsengine/defaults.go | 2 +- pkg/acsengine/output.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/acsengine/defaults.go b/pkg/acsengine/defaults.go index 09e8bf658a..624442bfba 100644 --- a/pkg/acsengine/defaults.go +++ b/pkg/acsengine/defaults.go @@ -757,7 +757,7 @@ func setDefaultCerts(p *api.Properties) (bool, error) { var err error caPair, err = helpers.CreatePkiKeyCertPair("ca") if err != nil { - return false, nil + return false, err } p.CertificateProfile.CaCertificate = caPair.CertificatePem p.CertificateProfile.CaPrivateKey = caPair.PrivateKeyPem diff --git a/pkg/acsengine/output.go b/pkg/acsengine/output.go index 0916ef352b..67da3eb065 100644 --- a/pkg/acsengine/output.go +++ b/pkg/acsengine/output.go @@ -2,7 +2,6 @@ package acsengine import ( "fmt" - "github.com/Azure/acs-engine/pkg/helpers" "io/ioutil" "path" "path/filepath"