Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

notary delete CLI command #895

Merged
merged 2 commits into from
Aug 8, 2016
Merged

notary delete CLI command #895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df1c425
notary delete CLI command and integration test
riyazdf Aug 2, 2016
2beb1fd
Factor out cert generation to helper
riyazdf Aug 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
267 changes: 184 additions & 83 deletions cmd/notary/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
"crypto/rand"
"crypto/sha256"
"crypto/sha512"
"crypto/x509"
"encoding/hex"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand All @@ -24,10 +26,12 @@ import (
"github.com/Sirupsen/logrus"
ctxu "github.com/docker/distribution/context"
"github.com/docker/notary"
"github.com/docker/notary/client"
"github.com/docker/notary/cryptoservice"
"github.com/docker/notary/passphrase"
"github.com/docker/notary/server"
"github.com/docker/notary/server/storage"
notaryStorage "github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/utils"
Expand Down Expand Up @@ -245,6 +249,148 @@ func TestClientTUFInteraction(t *testing.T) {
require.False(t, strings.Contains(string(output), target))
}

func TestClientDeleteTUFInteraction(t *testing.T) {
// -- setup --
setUp(t)

tempDir := tempDirWithConfig(t, "{}")
defer os.RemoveAll(tempDir)

server := setupServer()
defer server.Close()

tempFile, err := ioutil.TempFile("", "targetfile")
require.NoError(t, err)
tempFile.Close()
defer os.Remove(tempFile.Name())

// Setup certificate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking nitpick - setting up the cert and adding a delegation role + target to the delegation seems to also be done by TestPurge and TestClientDelegationsInteraction and TestClientDelegationsPublishing, although this one doesn't need the key ID of the cert. I was wondering if we could factor this logic out to a helper function?

Copy link
Contributor Author

@riyazdf riyazdf Aug 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea - added a helper to generate a private key, cert, and keyID

certFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)
cert, _, _ := generateCertPrivKeyPair(t, "gun", data.ECDSAKey)
_, err = certFile.Write(utils.CertToPEM(cert))
defer os.Remove(certFile.Name())

var (
output string
target = "helloIamanotarytarget"
)
// -- tests --

// init repo
_, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun")
require.NoError(t, err)

// add a target
_, err = runCommand(t, tempDir, "add", "gun", target, tempFile.Name())
require.NoError(t, err)

// check status - see target
output, err = runCommand(t, tempDir, "status", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(output, target))

// publish repo
_, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun")
require.NoError(t, err)

// list repo - see target
output, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(string(output), target))

// add a delegation and publish
output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", certFile.Name())
require.NoError(t, err)
_, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun")
require.NoError(t, err)

// list delegations - see role
output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(string(output), "targets/delegation"))

// Delete the repo metadata locally, so no need for server URL
output, err = runCommand(t, tempDir, "delete", "gun")
require.NoError(t, err)
assertLocalMetadataForGun(t, tempDir, "gun", false)

// list repo - see target still because remote data exists
output, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(string(output), target))

// list delegations - see role because remote data still exists
output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(string(output), "targets/delegation"))

// Trying to delete the repo with the remote flag fails if it's given a badly formed URL
output, err = runCommand(t, tempDir, "-s", "//invalidURLType", "delete", "gun", "--remote")
require.Error(t, err)
// since the connection fails to parse the URL before we can delete anything, local data should exist
assertLocalMetadataForGun(t, tempDir, "gun", true)

// Trying to delete the repo with the remote flag fails if it's given a well-formed URL that doesn't point to a server
output, err = runCommand(t, tempDir, "-s", "https://invalid-server", "delete", "gun", "--remote")
require.Error(t, err)
require.IsType(t, notaryStorage.ErrOffline{}, err)
// In this case, local notary metadata does not exist since local deletion operates first if we have a valid transport
assertLocalMetadataForGun(t, tempDir, "gun", false)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to assert that after this command the local metadata is still there (since getTransport fails before we make the call to DeleteTrustData)?

Copy link
Contributor Author

@riyazdf riyazdf Aug 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, I'll add this. Edit: actually, getTransport doesn't fail first, because it will only fail on invalid URL parsing. I can add a test case that catches this, though.

Copy link
Contributor

@cyli cyli Aug 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, sorry, I got confused -getTransport I guess only tries to make a connection if auth is set up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've differentiated the cases for an invalid URL vs. a well-formed URL that doesn't actually point to notary in the tests so we have this tracked

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thank you!

// Delete the repo remotely and locally, pointing to the correct server
output, err = runCommand(t, tempDir, "-s", server.URL, "delete", "gun", "--remote")
require.NoError(t, err)
assertLocalMetadataForGun(t, tempDir, "gun", false)
_, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun")
require.Error(t, err)
require.IsType(t, client.ErrRepositoryNotExist{}, err)

// Silent success on extraneous deletes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome detail on the tests!

output, err = runCommand(t, tempDir, "-s", server.URL, "delete", "gun", "--remote")
require.NoError(t, err)
assertLocalMetadataForGun(t, tempDir, "gun", false)
_, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun")
require.Error(t, err)
require.IsType(t, client.ErrRepositoryNotExist{}, err)

// Now check that we can re-publish the same repo
// init repo
_, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun")
require.NoError(t, err)

// add a target
_, err = runCommand(t, tempDir, "add", "gun", target, tempFile.Name())
require.NoError(t, err)

// check status - see target
output, err = runCommand(t, tempDir, "status", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(output, target))

// publish repo
_, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun")
require.NoError(t, err)

// list repo - see target
output, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun")
require.NoError(t, err)
require.True(t, strings.Contains(string(output), target))
}

func assertLocalMetadataForGun(t *testing.T, configDir, gun string, shouldExist bool) {
for _, role := range data.BaseRoles {
fileInfo, err := os.Stat(filepath.Join(configDir, "tuf", gun, "metadata", role+".json"))
if shouldExist {
require.NoError(t, err)
require.NotNil(t, fileInfo)
} else {
require.Error(t, err)
require.Nil(t, fileInfo)
}
}
}

// Initializes a repo, adds a target, publishes the target by hash, lists the target,
// verifies the target, and then removes the target.
func TestClientTUFAddByHashInteraction(t *testing.T) {
Expand Down Expand Up @@ -405,23 +551,12 @@ func TestClientDelegationsInteraction(t *testing.T) {
// Setup certificate
tempFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)

privKey, err := utils.GenerateECDSAKey(rand.Reader)
startTime := time.Now()
endTime := startTime.AddDate(10, 0, 0)
cert, err := cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
require.NoError(t, err)

cert, _, keyID := generateCertPrivKeyPair(t, "gun", data.ECDSAKey)
_, err = tempFile.Write(utils.CertToPEM(cert))
require.NoError(t, err)
tempFile.Close()
defer os.Remove(tempFile.Name())

rawPubBytes, _ := ioutil.ReadFile(tempFile.Name())
parsedPubKey, _ := utils.ParsePEMPublicKey(rawPubBytes)
keyID, err := utils.CanonicalKeyID(parsedPubKey)
require.NoError(t, err)

var output string

// -- tests --
Expand Down Expand Up @@ -494,23 +629,12 @@ func TestClientDelegationsInteraction(t *testing.T) {
tempFile2, err := ioutil.TempFile("", "pemfile2")
require.NoError(t, err)

privKey, err = utils.GenerateECDSAKey(rand.Reader)
startTime = time.Now()
endTime = startTime.AddDate(10, 0, 0)
cert, err = cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
require.NoError(t, err)

_, err = tempFile2.Write(utils.CertToPEM(cert))
require.NoError(t, err)
cert2, _, keyID2 := generateCertPrivKeyPair(t, "gun", data.ECDSAKey)
_, err = tempFile2.Write(utils.CertToPEM(cert2))
require.NoError(t, err)
tempFile2.Close()
defer os.Remove(tempFile2.Name())

rawPubBytes2, _ := ioutil.ReadFile(tempFile2.Name())
parsedPubKey2, _ := utils.ParsePEMPublicKey(rawPubBytes2)
keyID2, err := utils.CanonicalKeyID(parsedPubKey2)
require.NoError(t, err)

// add to the delegation by specifying the same role, this time add a scoped path
output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile2.Name(), "--paths", "path")
require.NoError(t, err)
Expand Down Expand Up @@ -777,26 +901,15 @@ func TestClientDelegationsPublishing(t *testing.T) {
// Setup certificate for delegation role
tempFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)

privKey, err := utils.GenerateRSAKey(rand.Reader, 2048)
require.NoError(t, err)
privKeyBytesNoRole, err := utils.KeyToPEM(privKey, "")
require.NoError(t, err)
privKeyBytesWithRole, err := utils.KeyToPEM(privKey, "user")
require.NoError(t, err)
startTime := time.Now()
endTime := startTime.AddDate(10, 0, 0)
cert, err := cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
require.NoError(t, err)

cert, privKey, canonicalKeyID := generateCertPrivKeyPair(t, "gun", data.RSAKey)
_, err = tempFile.Write(utils.CertToPEM(cert))
require.NoError(t, err)
tempFile.Close()
defer os.Remove(tempFile.Name())

rawPubBytes, _ := ioutil.ReadFile(tempFile.Name())
parsedPubKey, _ := utils.ParsePEMPublicKey(rawPubBytes)
canonicalKeyID, err := utils.CanonicalKeyID(parsedPubKey)
privKeyBytesNoRole, err := utils.KeyToPEM(privKey, "")
require.NoError(t, err)
privKeyBytesWithRole, err := utils.KeyToPEM(privKey, "user")
require.NoError(t, err)

// Set up targets for publishing
Expand Down Expand Up @@ -951,24 +1064,12 @@ func TestClientDelegationsPublishing(t *testing.T) {
// Setup another certificate
tempFile2, err := ioutil.TempFile("", "pemfile2")
require.NoError(t, err)

privKey, err = utils.GenerateECDSAKey(rand.Reader)
startTime = time.Now()
endTime = startTime.AddDate(10, 0, 0)
cert, err = cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
require.NoError(t, err)

_, err = tempFile2.Write(utils.CertToPEM(cert))
require.NoError(t, err)
cert2, _, keyID2 := generateCertPrivKeyPair(t, "gun", data.RSAKey)
_, err = tempFile2.Write(utils.CertToPEM(cert2))
require.NoError(t, err)
tempFile2.Close()
defer os.Remove(tempFile2.Name())

rawPubBytes2, _ := ioutil.ReadFile(tempFile2.Name())
parsedPubKey2, _ := utils.ParsePEMPublicKey(rawPubBytes2)
keyID2, err := utils.CanonicalKeyID(parsedPubKey2)
require.NoError(t, err)

// add a nested delegation under this releases role
output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/releases/nested", tempFile2.Name(), "--paths", "nested/path")
require.NoError(t, err)
Expand Down Expand Up @@ -1171,21 +1272,6 @@ func TestClientKeyGenerationRotation(t *testing.T) {
require.True(t, strings.Contains(string(output), target))
}

// Helper method to get the subdirectory for TUF keys
func getKeySubdir(role, gun string) string {
subdir := notary.PrivDir
switch role {
case data.CanonicalRootRole:
return filepath.Join(subdir, notary.RootKeysSubdir)
case data.CanonicalTargetsRole:
return filepath.Join(subdir, notary.NonRootKeysSubdir, gun)
case data.CanonicalSnapshotRole:
return filepath.Join(subdir, notary.NonRootKeysSubdir, gun)
default:
return filepath.Join(subdir, notary.NonRootKeysSubdir)
}
}

// Tests default root key generation
func TestDefaultRootKeyGeneration(t *testing.T) {
// -- setup --
Expand Down Expand Up @@ -1388,29 +1474,21 @@ func TestWitness(t *testing.T) {
server := setupServer()
defer server.Close()

startTime := time.Now()
endTime := startTime.AddDate(10, 0, 0)

// Setup certificates
tempFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)
privKey, err := utils.GenerateECDSAKey(rand.Reader)
cert, err := cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
require.NoError(t, err)

cert, privKey, keyID := generateCertPrivKeyPair(t, "gun", data.ECDSAKey)
_, err = tempFile.Write(utils.CertToPEM(cert))
require.NoError(t, err)
tempFile.Close()
defer os.Remove(tempFile.Name())
rawPubBytes, _ := ioutil.ReadFile(tempFile.Name())
parsedPubKey, _ := utils.ParsePEMPublicKey(rawPubBytes)
keyID, err := utils.CanonicalKeyID(parsedPubKey)
require.NoError(t, err)

tempFile2, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)
privKey2, err := utils.GenerateECDSAKey(rand.Reader)
cert2, err := cryptoservice.GenerateCertificate(privKey2, "gun", startTime, endTime)
// Setup another certificate
tempFile2, err := ioutil.TempFile("", "pemfile2")
require.NoError(t, err)

cert2, privKey2, _ := generateCertPrivKeyPair(t, "gun", data.ECDSAKey)
_, err = tempFile2.Write(utils.CertToPEM(cert2))
require.NoError(t, err)
tempFile2.Close()
Expand Down Expand Up @@ -1526,3 +1604,26 @@ func TestWitness(t *testing.T) {
require.Error(t, err)
}
}

func generateCertPrivKeyPair(t *testing.T, gun, keyAlgorithm string) (*x509.Certificate, data.PrivateKey, string) {
// Setup certificate
var privKey data.PrivateKey
var err error
switch keyAlgorithm {
case data.ECDSAKey:
privKey, err = utils.GenerateECDSAKey(rand.Reader)
case data.RSAKey:
privKey, err = utils.GenerateRSAKey(rand.Reader, 4096)
default:
err = fmt.Errorf("invalid key algorithm provided: %s", keyAlgorithm)
}
require.NoError(t, err)
startTime := time.Now()
endTime := startTime.AddDate(10, 0, 0)
cert, err := cryptoservice.GenerateCertificate(privKey, gun, startTime, endTime)
require.NoError(t, err)
parsedPubKey, _ := utils.ParsePEMPublicKey(utils.CertToPEM(cert))
keyID, err := utils.CanonicalKeyID(parsedPubKey)
require.NoError(t, err)
return cert, privKey, keyID
}
1 change: 1 addition & 0 deletions cmd/notary/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ var exampleValidCommands = []string{
"delegation add repo targets/releases path/to/pem/file.pem",
"delegation remove repo targets/releases",
"witness gun targets/releases",
"delete repo",
}

// config parsing bugs are propagated in all commands
Expand Down
Loading