-
Notifications
You must be signed in to change notification settings - Fork 511
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.
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
notary delete CLI command #895
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,9 @@ import ( | |
"crypto/rand" | ||
"crypto/sha256" | ||
"crypto/sha512" | ||
"crypto/x509" | ||
"encoding/hex" | ||
"fmt" | ||
"io/ioutil" | ||
"net/http" | ||
"net/http/httptest" | ||
|
@@ -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" | ||
|
@@ -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 | ||
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) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right, sorry, I got confused - There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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 -- | ||
|
@@ -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) | ||
|
@@ -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 | ||
|
@@ -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) | ||
|
@@ -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 -- | ||
|
@@ -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() | ||
|
@@ -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 | ||
} |
There was a problem hiding this comment.
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
andTestClientDelegationsInteraction
andTestClientDelegationsPublishing
, 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?There was a problem hiding this comment.
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