From a3fd061c02fcd2ccc1112595a564f28839596915 Mon Sep 17 00:00:00 2001 From: Jeremy Rickard Date: Wed, 5 Jun 2019 09:39:52 -0600 Subject: [PATCH 1/3] Add a call to MkdirAll for the credentials directory path. This adds a call to MkdirAll with the credentials path before we attempt to write the credential file. This will ensure that the credential directory exists before we try and write the file. If the directory exists already, it's a NO-OP. Fixes #377 --- pkg/porter/credentials.go | 12 +++++++- pkg/porter/credentials_test.go | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/pkg/porter/credentials.go b/pkg/porter/credentials.go index d3e62715b..5f2289a94 100644 --- a/pkg/porter/credentials.go +++ b/pkg/porter/credentials.go @@ -135,6 +135,9 @@ func (g *CredentialOptions) validateCredName(args []string) error { return nil } +// GenerateCredentials builds a new credential set based on the given options. This can be either +// a silent build, based on the opts.Silent flag, or interactive using a survey. Returns an +// error if unable to generate credentials func (p *Porter) GenerateCredentials(opts CredentialOptions) error { //TODO make this work for either porter.yaml OR a bundle @@ -168,13 +171,20 @@ func (p *Porter) GenerateCredentials(opts CredentialOptions) error { fmt.Fprintf(p.Out, "%v", string(data)) return nil } - + credentialsDir, err := p.Config.GetCredentialsDir() + if err != nil { + return errors.Wrap(err, "unable to get credentials directory") + } + // Make the credentials path if it doesn't exist. MkdirAll does nothing if it already exists + // Readable, writable only by the user + err = p.Config.FileSystem.MkdirAll(credentialsDir, 0700) dest, err := p.Config.GetCredentialPath(genOpts.Name) if err != nil { return errors.Wrap(err, "unable to determine credentials directory") } fmt.Fprintf(p.Out, "Saving credential to %s\n", dest) + err = p.Context.FileSystem.WriteFile(dest, data, 0600) if err != nil { return errors.Wrapf(err, "couldn't write credential file %s", dest) diff --git a/pkg/porter/credentials_test.go b/pkg/porter/credentials_test.go index e8d9ca350..f1eb8250d 100644 --- a/pkg/porter/credentials_test.go +++ b/pkg/porter/credentials_test.go @@ -7,6 +7,7 @@ import ( printer "github.com/deislabs/porter/pkg/printer" "github.com/deislabs/duffle/pkg/bundle" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -231,3 +232,57 @@ good-creds now`}, }) } } + +func TestGenerateNoCredentialDirectory(t *testing.T) { + p := NewTestPorter(t) + p.TestConfig.SetupPorterHome() + p.CNAB = &TestCNABProvider{} + + opts := CredentialOptions{ + Silent: true, + } + opts.Name = "name" + credDir, err := p.Config.GetCredentialsDir() + require.NoError(t, err, "should have been able to get credentials directory path") + credDirExists, err := p.Porter.Context.FileSystem.DirExists(credDir) + require.NoError(t, err, "shouldn't have failed on dir exists") + require.False(t, credDirExists, "there should not have been a credential directory for this test") + err = p.GenerateCredentials(opts) + assert.NoError(t, err, "credential generation should have been successful") + credDirExists, err = p.Porter.Context.FileSystem.DirExists(credDir) + assert.NoError(t, err, "shouldn't have gotten an error checking credential directory after generate") + assert.True(t, credDirExists, "should have been a credential directory after the generation") + path, err := p.Porter.Config.GetCredentialPath("name") + assert.NoError(t, err, "couldn't get credential path") + credFileExists, err := p.Porter.Context.FileSystem.Exists(path) + assert.True(t, credFileExists, "expected the file %s to exist", path) + assert.NoError(t, err, "should have been able to check if get credential path exists") +} + +func TestGenerateCredentialDirectoryExists(t *testing.T) { + p := NewTestPorter(t) + p.TestConfig.SetupPorterHome() + p.CNAB = &TestCNABProvider{} + + opts := CredentialOptions{ + Silent: true, + } + opts.Name = "name" + credDir, err := p.Config.GetCredentialsDir() + require.NoError(t, err, "should have been able to get credentials directory path") + err = p.Config.FileSystem.MkdirAll(credDir, 0600) + require.NoError(t, err, "should have been able to make directory path") + credDirExists, err := p.Porter.Context.FileSystem.DirExists(credDir) + require.NoError(t, err, "shouldn't have failed on dir exists") + require.True(t, credDirExists, "there should have been a credential directory for this test") + err = p.GenerateCredentials(opts) + assert.NoError(t, err, "credential generation should have been successful") + credDirExists, err = p.Porter.Context.FileSystem.DirExists(credDir) + assert.NoError(t, err, "shouldn't have gotten an error checking credential directory after generate") + assert.True(t, credDirExists, "should have been a credential directory after the generation") + path, err := p.Porter.Config.GetCredentialPath("name") + assert.NoError(t, err, "couldn't get credential path") + credFileExists, err := p.Porter.Context.FileSystem.Exists(path) + assert.True(t, credFileExists, "expected the file %s to exist", path) + assert.NoError(t, err, "should have been able to check if get credential path exists") +} From ca01907ad215fc81b8680bf272ecde1e5231c41a Mon Sep 17 00:00:00 2001 From: Jeremy Rickard Date: Wed, 5 Jun 2019 10:20:22 -0600 Subject: [PATCH 2/3] Oopps didn't handle the error --- pkg/porter/credentials.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/porter/credentials.go b/pkg/porter/credentials.go index 5f2289a94..aeb141825 100644 --- a/pkg/porter/credentials.go +++ b/pkg/porter/credentials.go @@ -178,9 +178,12 @@ func (p *Porter) GenerateCredentials(opts CredentialOptions) error { // Make the credentials path if it doesn't exist. MkdirAll does nothing if it already exists // Readable, writable only by the user err = p.Config.FileSystem.MkdirAll(credentialsDir, 0700) + if err != nil { + return errors.Wrap(err, "unable to create credentials directory") + } dest, err := p.Config.GetCredentialPath(genOpts.Name) if err != nil { - return errors.Wrap(err, "unable to determine credentials directory") + return errors.Wrap(err, "unable to determine credentials path") } fmt.Fprintf(p.Out, "Saving credential to %s\n", dest) From af090d62330d4f91607a562cfb0bcd38fa22776c Mon Sep 17 00:00:00 2001 From: Jeremy Rickard Date: Wed, 5 Jun 2019 10:22:59 -0600 Subject: [PATCH 3/3] Cleanup test for readability --- pkg/porter/credentials_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/porter/credentials_test.go b/pkg/porter/credentials_test.go index f1eb8250d..f60cfec69 100644 --- a/pkg/porter/credentials_test.go +++ b/pkg/porter/credentials_test.go @@ -242,16 +242,23 @@ func TestGenerateNoCredentialDirectory(t *testing.T) { Silent: true, } opts.Name = "name" + + //Check if the credentials directory exists in the FS. It shouldn't. credDir, err := p.Config.GetCredentialsDir() require.NoError(t, err, "should have been able to get credentials directory path") credDirExists, err := p.Porter.Context.FileSystem.DirExists(credDir) require.NoError(t, err, "shouldn't have failed on dir exists") require.False(t, credDirExists, "there should not have been a credential directory for this test") + + //Now generate the credentials. After completion, the directory should now exist. It should be + //created if it does not exit err = p.GenerateCredentials(opts) assert.NoError(t, err, "credential generation should have been successful") credDirExists, err = p.Porter.Context.FileSystem.DirExists(credDir) assert.NoError(t, err, "shouldn't have gotten an error checking credential directory after generate") assert.True(t, credDirExists, "should have been a credential directory after the generation") + + //Verify that the credential was actually created. path, err := p.Porter.Config.GetCredentialPath("name") assert.NoError(t, err, "couldn't get credential path") credFileExists, err := p.Porter.Context.FileSystem.Exists(path) @@ -268,18 +275,26 @@ func TestGenerateCredentialDirectoryExists(t *testing.T) { Silent: true, } opts.Name = "name" + + //Create the credentials directory credDir, err := p.Config.GetCredentialsDir() require.NoError(t, err, "should have been able to get credentials directory path") err = p.Config.FileSystem.MkdirAll(credDir, 0600) require.NoError(t, err, "should have been able to make directory path") + + //Verify the directory does in fact, exist. credDirExists, err := p.Porter.Context.FileSystem.DirExists(credDir) require.NoError(t, err, "shouldn't have failed on dir exists") require.True(t, credDirExists, "there should have been a credential directory for this test") + + //Generate the credential now. The directory does exist, so there should be no error. err = p.GenerateCredentials(opts) assert.NoError(t, err, "credential generation should have been successful") credDirExists, err = p.Porter.Context.FileSystem.DirExists(credDir) assert.NoError(t, err, "shouldn't have gotten an error checking credential directory after generate") assert.True(t, credDirExists, "should have been a credential directory after the generation") + + //Verify we wrote the credential file. path, err := p.Porter.Config.GetCredentialPath("name") assert.NoError(t, err, "couldn't get credential path") credFileExists, err := p.Porter.Context.FileSystem.Exists(path)