Skip to content

Commit 1981706

Browse files
thaJeztahthaJeztah
committed
daemon: remove migrateTrustKeyID()
The migration code is in the 22.06 branch, and if we don't migrate the only side-effect is the daemon's ID being regenerated (as a UUID). Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 66ca24d commit 1981706

4 files changed

Lines changed: 9 additions & 154 deletions

File tree

‎daemon/daemon.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -931,14 +931,6 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
931931
return nil, err
932932
}
933933

934-
// Try to preserve the daemon ID (which is the trust-key's ID) when upgrading
935-
// an existing installation; this is a "best-effort".
936-
idPath := filepath.Join(config.Root, "engine-id")
937-
err = migrateTrustKeyID(config.TrustKeyPath, idPath)
938-
if err != nil {
939-
logrus.WithError(err).Warnf("unable to migrate engine ID; a new engine ID will be generated")
940-
}
941-
942934
// Check if Devices cgroup is mounted, it is hard requirement for container security,
943935
// on Linux.
944936
//
@@ -951,7 +943,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
951943
return nil, errors.New("Devices cgroup isn't mounted")
952944
}
953945

954-
d.id, err = loadOrCreateID(idPath)
946+
d.id, err = loadOrCreateID(filepath.Join(config.Root, "engine-id"))
955947
if err != nil {
956948
return nil, err
957949
}

‎daemon/id.go‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ import (
44
"os"
55

66
"github.com/docker/docker/pkg/ioutils"
7-
"github.com/docker/libtrust"
87
"github.com/google/uuid"
98
"github.com/pkg/errors"
10-
"github.com/sirupsen/logrus"
119
)
1210

1311
// loadOrCreateID loads the engine's ID from idPath, or generates a new ID
@@ -32,30 +30,3 @@ func loadOrCreateID(idPath string) (string, error) {
3230
}
3331
return id, nil
3432
}
35-
36-
// migrateTrustKeyID migrates the daemon ID of existing installations. It returns
37-
// an error when a trust-key was found, but we failed to read it, or failed to
38-
// complete the migration.
39-
//
40-
// We migrate the ID so that engines don't get a new ID generated on upgrades,
41-
// which may be unexpected (and users may be using the ID for various purposes).
42-
func migrateTrustKeyID(deprecatedTrustKeyPath, idPath string) error {
43-
if _, err := os.Stat(idPath); err == nil {
44-
// engine ID file already exists; no migration needed
45-
return nil
46-
}
47-
trustKey, err := libtrust.LoadKeyFile(deprecatedTrustKeyPath)
48-
if err != nil {
49-
if err == libtrust.ErrKeyFileDoesNotExist {
50-
// no existing trust-key found; no migration needed
51-
return nil
52-
}
53-
return err
54-
}
55-
id := trustKey.PublicKey().KeyID()
56-
if err := ioutils.AtomicWriteFile(idPath, []byte(id), os.FileMode(0600)); err != nil {
57-
return errors.Wrap(err, "error saving ID file")
58-
}
59-
logrus.Info("successfully migrated engine ID")
60-
return nil
61-
}

‎integration-cli/docker_cli_daemon_test.go‎

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
"github.com/docker/docker/opts"
3636
testdaemon "github.com/docker/docker/testutil/daemon"
3737
units "github.com/docker/go-units"
38-
"github.com/docker/libtrust"
3938
"github.com/moby/sys/mount"
4039
"golang.org/x/sys/unix"
4140
"gotest.tools/v3/assert"
@@ -556,24 +555,6 @@ func (s *DockerDaemonSuite) TestDaemonAllocatesListeningPort(c *testing.T) {
556555
}
557556
}
558557

559-
func (s *DockerDaemonSuite) TestDaemonKeyGeneration(c *testing.T) {
560-
// TODO: skip or update for Windows daemon
561-
os.Remove("/etc/docker/key.json")
562-
c.Setenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE", "1")
563-
s.d.Start(c)
564-
s.d.Stop(c)
565-
566-
k, err := libtrust.LoadKeyFile("/etc/docker/key.json")
567-
if err != nil {
568-
c.Fatalf("Error opening key file")
569-
}
570-
kid := k.KeyID()
571-
// Test Key ID is a valid fingerprint (e.g. QQXN:JY5W:TBXI:MK3X:GX6P:PD5D:F56N:NHCS:LVRZ:JA46:R24J:XEFF)
572-
if len(kid) != 59 {
573-
c.Fatalf("Bad key ID: %s", kid)
574-
}
575-
}
576-
577558
// GH#11320 - verify that the daemon exits on failure properly
578559
// Note that this explicitly tests the conflict of {-b,--bridge} and {--bip} options as the means
579560
// to get a daemon init failure; no other tests for -b/--bip conflict are therefore required
@@ -1201,60 +1182,6 @@ func (s *DockerDaemonSuite) TestDaemonUnixSockCleanedUp(c *testing.T) {
12011182
}
12021183
}
12031184

1204-
func (s *DockerDaemonSuite) TestDaemonWithWrongkey(c *testing.T) {
1205-
type Config struct {
1206-
Crv string `json:"crv"`
1207-
D string `json:"d"`
1208-
Kid string `json:"kid"`
1209-
Kty string `json:"kty"`
1210-
X string `json:"x"`
1211-
Y string `json:"y"`
1212-
}
1213-
1214-
os.Remove("/etc/docker/key.json")
1215-
c.Setenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE", "1")
1216-
s.d.Start(c)
1217-
s.d.Stop(c)
1218-
1219-
config := &Config{}
1220-
bytes, err := os.ReadFile("/etc/docker/key.json")
1221-
if err != nil {
1222-
c.Fatalf("Error reading key.json file: %s", err)
1223-
}
1224-
1225-
// byte[] to Data-Struct
1226-
if err := json.Unmarshal(bytes, &config); err != nil {
1227-
c.Fatalf("Error Unmarshal: %s", err)
1228-
}
1229-
1230-
// replace config.Kid with the fake value
1231-
config.Kid = "VSAJ:FUYR:X3H2:B2VZ:KZ6U:CJD5:K7BX:ZXHY:UZXT:P4FT:MJWG:HRJ4"
1232-
1233-
// NEW Data-Struct to byte[]
1234-
newBytes, err := json.Marshal(&config)
1235-
if err != nil {
1236-
c.Fatalf("Error Marshal: %s", err)
1237-
}
1238-
1239-
// write back
1240-
if err := os.WriteFile("/etc/docker/key.json", newBytes, 0400); err != nil {
1241-
c.Fatalf("Error os.WriteFile: %s", err)
1242-
}
1243-
1244-
defer os.Remove("/etc/docker/key.json")
1245-
1246-
if err := s.d.StartWithError(); err == nil {
1247-
c.Fatalf("It should not be successful to start daemon with wrong key: %v", err)
1248-
}
1249-
1250-
content, err := s.d.ReadLogFile()
1251-
assert.Assert(c, err == nil)
1252-
1253-
if !strings.Contains(string(content), "Public Key ID does not match") {
1254-
c.Fatalf("Missing KeyID message from daemon logs: %s", string(content))
1255-
}
1256-
}
1257-
12581185
func (s *DockerDaemonSuite) TestDaemonRestartKillWait(c *testing.T) {
12591186
s.d.StartWithBusybox(c)
12601187

‎integration/daemon/daemon_test.go‎

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -24,62 +24,27 @@ import (
2424
"gotest.tools/v3/skip"
2525
)
2626

27-
const (
28-
libtrustKey = `{"crv":"P-256","d":"dm28PH4Z4EbyUN8L0bPonAciAQa1QJmmyYd876mnypY","kid":"WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB","kty":"EC","x":"Mh5-JINSjaa_EZdXDttri255Z5fbCEOTQIZjAcScFTk","y":"eUyuAjfxevb07hCCpvi4Zi334Dy4GDWQvEToGEX4exQ"}`
29-
libtrustKeyID = "WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB"
30-
)
31-
32-
func TestConfigDaemonLibtrustID(t *testing.T) {
33-
skip.If(t, runtime.GOOS == "windows")
34-
35-
d := daemon.New(t)
36-
defer d.Stop(t)
37-
38-
trustKey := filepath.Join(d.RootDir(), "key.json")
39-
err := os.WriteFile(trustKey, []byte(libtrustKey), 0644)
40-
assert.NilError(t, err)
41-
42-
cfg := filepath.Join(d.RootDir(), "daemon.json")
43-
err = os.WriteFile(cfg, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644)
44-
assert.NilError(t, err)
45-
46-
d.Start(t, "--config-file", cfg)
47-
info := d.Info(t)
48-
assert.Equal(t, info.ID, libtrustKeyID)
49-
}
50-
5127
func TestConfigDaemonID(t *testing.T) {
5228
skip.If(t, runtime.GOOS == "windows")
5329

5430
d := daemon.New(t)
5531
defer d.Stop(t)
5632

57-
trustKey := filepath.Join(d.RootDir(), "key.json")
58-
err := os.WriteFile(trustKey, []byte(libtrustKey), 0644)
59-
assert.NilError(t, err)
60-
61-
cfg := filepath.Join(d.RootDir(), "daemon.json")
62-
err = os.WriteFile(cfg, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644)
63-
assert.NilError(t, err)
64-
65-
// Verify that on an installation with a trust-key present, the ID matches
66-
// the trust-key ID, and that the ID has been migrated to the engine-id file.
67-
d.Start(t, "--config-file", cfg, "--iptables=false")
33+
d.Start(t, "--iptables=false")
6834
info := d.Info(t)
69-
assert.Equal(t, info.ID, libtrustKeyID)
70-
71-
idFile := filepath.Join(d.RootDir(), "engine-id")
72-
id, err := os.ReadFile(idFile)
73-
assert.NilError(t, err)
74-
assert.Equal(t, string(id), libtrustKeyID)
35+
assert.Check(t, info.ID != "")
7536
d.Stop(t)
7637

7738
// Verify that (if present) the engine-id file takes precedence
7839
const engineID = "this-is-the-engine-id"
79-
err = os.WriteFile(idFile, []byte(engineID), 0600)
40+
idFile := filepath.Join(d.RootDir(), "engine-id")
41+
assert.Check(t, os.Remove(idFile))
42+
// Using 0644 to allow rootless daemons to read the file (ideally
43+
// we'd chown the file to have the remapped user as owner).
44+
err := os.WriteFile(idFile, []byte(engineID), 0o644)
8045
assert.NilError(t, err)
8146

82-
d.Start(t, "--config-file", cfg, "--iptables=false")
47+
d.Start(t, "--iptables=false")
8348
info = d.Info(t)
8449
assert.Equal(t, info.ID, engineID)
8550
d.Stop(t)

0 commit comments

Comments
 (0)