20485: Test handling of nil publickey argument. 20485-optional-deploy-ssh-key
authorTom Clegg <tom@curii.com>
Wed, 31 May 2023 19:55:33 +0000 (15:55 -0400)
committerTom Clegg <tom@curii.com>
Wed, 31 May 2023 19:55:33 +0000 (15:55 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/cloud/azure/azure_test.go
lib/cloud/ec2/ec2_test.go

index b6aa9a16b6187b4fd5486037076885fe4eb69d19..2f88f7344044e6e31d80e9ef00469771a0d0a116 100644 (file)
@@ -69,14 +69,17 @@ var _ = check.Suite(&AzureInstanceSetSuite{})
 
 const testNamePrefix = "compute-test123-"
 
-type VirtualMachinesClientStub struct{}
+type VirtualMachinesClientStub struct {
+       vmParameters compute.VirtualMachine
+}
 
-func (*VirtualMachinesClientStub) createOrUpdate(ctx context.Context,
+func (stub *VirtualMachinesClientStub) createOrUpdate(ctx context.Context,
        resourceGroupName string,
        VMName string,
        parameters compute.VirtualMachine) (result compute.VirtualMachine, err error) {
        parameters.ID = &VMName
        parameters.Name = &VMName
+       stub.vmParameters = parameters
        return parameters, nil
 }
 
@@ -124,7 +127,7 @@ type testConfig struct {
 
 var live = flag.String("live-azure-cfg", "", "Test with real azure API, provide config file")
 
-func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error) {
+func GetInstanceSet() (*azureInstanceSet, cloud.ImageID, arvados.Cluster, error) {
        cluster := arvados.Cluster{
                InstanceTypes: arvados.InstanceTypeMap(map[string]arvados.InstanceType{
                        "tiny": {
@@ -154,7 +157,7 @@ func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error)
                }
 
                ap, err := newAzureInstanceSet(exampleCfg.DriverParameters, "test123", nil, logrus.StandardLogger())
-               return ap, cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster, err
+               return ap.(*azureInstanceSet), cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster, err
        }
        ap := azureInstanceSet{
                azconfig: azureInstanceSetConfig{
@@ -193,18 +196,25 @@ func (*AzureInstanceSetSuite) TestCreate(c *check.C) {
        tags := inst.Tags()
        c.Check(tags["TestTagName"], check.Equals, "test tag value")
        c.Logf("inst.String()=%v Address()=%v Tags()=%v", inst.String(), inst.Address(), tags)
+       if *live == "" {
+               c.Check(ap.vmClient.(*VirtualMachinesClientStub).vmParameters.VirtualMachineProperties.OsProfile.LinuxConfiguration.SSH, check.NotNil)
+       }
 
        instPreemptable, err := ap.Create(cluster.InstanceTypes["tinyp"],
                img, map[string]string{
                        "TestTagName": "test tag value",
-               }, "umask 0600; echo -n test-file-data >/var/run/test-file", pk)
+               }, "umask 0600; echo -n test-file-data >/var/run/test-file", nil)
 
        c.Assert(err, check.IsNil)
 
        tags = instPreemptable.Tags()
        c.Check(tags["TestTagName"], check.Equals, "test tag value")
        c.Logf("instPreemptable.String()=%v Address()=%v Tags()=%v", instPreemptable.String(), instPreemptable.Address(), tags)
-
+       if *live == "" {
+               // Should not have set SSH option, because publickey
+               // arg was nil
+               c.Check(ap.vmClient.(*VirtualMachinesClientStub).vmParameters.VirtualMachineProperties.OsProfile.LinuxConfiguration.SSH, check.IsNil)
+       }
 }
 
 func (*AzureInstanceSetSuite) TestListInstances(c *check.C) {
@@ -229,7 +239,7 @@ func (*AzureInstanceSetSuite) TestManageNics(c *check.C) {
                c.Fatal("Error making provider", err)
        }
 
-       ap.(*azureInstanceSet).manageNics()
+       ap.manageNics()
        ap.Stop()
 }
 
@@ -239,7 +249,7 @@ func (*AzureInstanceSetSuite) TestManageBlobs(c *check.C) {
                c.Fatal("Error making provider", err)
        }
 
-       ap.(*azureInstanceSet).manageBlobs()
+       ap.manageBlobs()
        ap.Stop()
 }
 
@@ -263,7 +273,7 @@ func (*AzureInstanceSetSuite) TestDeleteFake(c *check.C) {
                c.Fatal("Error making provider", err)
        }
 
-       _, err = ap.(*azureInstanceSet).netClient.delete(context.Background(), "fakefakefake", "fakefakefake")
+       _, err = ap.netClient.delete(context.Background(), "fakefakefake", "fakefakefake")
 
        de, ok := err.(autorest.DetailedError)
        if ok {
index 38ada13ed3f5f14fe781e548727745d5fd079c4e..ede7f9de5d2cc8b3dd1ddb17416782c3ec21aee7 100644 (file)
@@ -57,15 +57,19 @@ type testConfig struct {
 }
 
 type ec2stub struct {
-       c       *check.C
-       reftime time.Time
+       c                     *check.C
+       reftime               time.Time
+       importKeyPairCalls    []*ec2.ImportKeyPairInput
+       describeKeyPairsCalls []*ec2.DescribeKeyPairsInput
 }
 
 func (e *ec2stub) ImportKeyPair(input *ec2.ImportKeyPairInput) (*ec2.ImportKeyPairOutput, error) {
+       e.importKeyPairCalls = append(e.importKeyPairCalls, input)
        return nil, nil
 }
 
 func (e *ec2stub) DescribeKeyPairs(input *ec2.DescribeKeyPairsInput) (*ec2.DescribeKeyPairsOutput, error) {
+       e.describeKeyPairsCalls = append(e.describeKeyPairsCalls, input)
        return &ec2.DescribeKeyPairsOutput{}, nil
 }
 
@@ -213,16 +217,18 @@ func (*EC2InstanceSetSuite) TestCreate(c *check.C) {
        c.Check(tags["TestTagName"], check.Equals, "test tag value")
        c.Logf("inst.String()=%v Address()=%v Tags()=%v", inst.String(), inst.Address(), tags)
 
+       if *live == "" {
+               c.Check(ap.client.(*ec2stub).describeKeyPairsCalls, check.HasLen, 1)
+               c.Check(ap.client.(*ec2stub).importKeyPairCalls, check.HasLen, 1)
+       }
 }
 
 func (*EC2InstanceSetSuite) TestCreateWithExtraScratch(c *check.C) {
        ap, img, cluster := GetInstanceSet(c)
-       pk, _ := test.LoadTestKey(c, "../../dispatchcloud/test/sshkey_dispatch")
-
        inst, err := ap.Create(cluster.InstanceTypes["tiny-with-extra-scratch"],
                img, map[string]string{
                        "TestTagName": "test tag value",
-               }, "umask 0600; echo -n test-file-data >/var/run/test-file", pk)
+               }, "umask 0600; echo -n test-file-data >/var/run/test-file", nil)
 
        c.Assert(err, check.IsNil)
 
@@ -230,6 +236,12 @@ func (*EC2InstanceSetSuite) TestCreateWithExtraScratch(c *check.C) {
        c.Check(tags["TestTagName"], check.Equals, "test tag value")
        c.Logf("inst.String()=%v Address()=%v Tags()=%v", inst.String(), inst.Address(), tags)
 
+       if *live == "" {
+               // Should not have called key pair APIs, because
+               // publickey arg was nil
+               c.Check(ap.client.(*ec2stub).describeKeyPairsCalls, check.HasLen, 0)
+               c.Check(ap.client.(*ec2stub).importKeyPairCalls, check.HasLen, 0)
+       }
 }
 
 func (*EC2InstanceSetSuite) TestCreatePreemptible(c *check.C) {