From 7daab31f1f3afe8828c22656482a7124b40e1f08 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 24 Mar 2023 11:34:31 -0400 Subject: [PATCH] Ensure that SQLite state handles name-ID collisions If a container with an ID starting with "db1" exists, and a container named "db1" also exists, and they are different containers - if I run `podman inspect db1` the container named "db1" should be inspected, and there should not be an error that multiple containers matched the name or id "db1". This was already handled by BoltDB, and now is properly managed by SQLite. Fixes #17905 Signed-off-by: Matt Heon --- libpod/sqlite_state.go | 81 ++++++++++++++++++++++--------------- test/e2e/create_test.go | 17 ++++++++ test/e2e/pod_create_test.go | 16 ++++++++ 3 files changed, 82 insertions(+), 32 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 9027083ddc..222c9edd61 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -487,26 +487,29 @@ func (s *SQLiteState) LookupContainerID(idOrName string) (string, error) { return "", define.ErrDBClosed } - rows, err := s.conn.Query("SELECT ID FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") + rows, err := s.conn.Query("SELECT ID, Name FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return "", fmt.Errorf("looking up container %q in database: %w", idOrName, err) } defer rows.Close() - var id string - foundResult := false + var ( + id, name string + resCount uint + ) for rows.Next() { - if foundResult { - return "", fmt.Errorf("more than one result for container %q: %w", idOrName, define.ErrCtrExists) - } - - if err := rows.Scan(&id); err != nil { + if err := rows.Scan(&id, &name); err != nil { return "", fmt.Errorf("retrieving container %q ID from database: %w", idOrName, err) } - foundResult = true + if name == idOrName { + return id, nil + } + resCount++ } - if !foundResult { + if resCount == 0 { return "", define.ErrNoSuchCtr + } else if resCount > 1 { + return "", fmt.Errorf("more than one result for container %q: %w", idOrName, define.ErrCtrExists) } return id, nil @@ -523,26 +526,33 @@ func (s *SQLiteState) LookupContainer(idOrName string) (*Container, error) { return nil, define.ErrDBClosed } - rows, err := s.conn.Query("SELECT JSON FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") + rows, err := s.conn.Query("SELECT JSON, Name FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return nil, fmt.Errorf("looking up container %q in database: %w", idOrName, err) } defer rows.Close() - var rawJSON string - foundResult := false + var ( + rawJSON, name string + exactName bool + resCount uint + ) for rows.Next() { - if foundResult { - return nil, fmt.Errorf("more than one result for container %q: %w", idOrName, define.ErrCtrExists) - } - - if err := rows.Scan(&rawJSON); err != nil { + if err := rows.Scan(&rawJSON, &name); err != nil { return nil, fmt.Errorf("retrieving container %q ID from database: %w", idOrName, err) } - foundResult = true + if name == idOrName { + exactName = true + break + } + resCount++ } - if !foundResult { - return nil, fmt.Errorf("no container with name or ID %q found: %w", idOrName, define.ErrNoSuchCtr) + if !exactName { + if resCount == 0 { + return nil, fmt.Errorf("no container with name or ID %q found: %w", idOrName, define.ErrNoSuchCtr) + } else if resCount > 1 { + return nil, fmt.Errorf("more than one result for container %q: %w", idOrName, define.ErrCtrExists) + } } ctr := new(Container) @@ -1303,26 +1313,33 @@ func (s *SQLiteState) LookupPod(idOrName string) (*Pod, error) { return nil, define.ErrDBClosed } - rows, err := s.conn.Query("SELECT JSON FROM PodConfig WHERE PodConfig.Name=? OR (PodConfig.ID LIKE ?);", idOrName, idOrName+"%") + rows, err := s.conn.Query("SELECT JSON, Name FROM PodConfig WHERE PodConfig.Name=? OR (PodConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return nil, fmt.Errorf("looking up pod %q in database: %w", idOrName, err) } defer rows.Close() - var rawJSON string - foundResult := false + var ( + rawJSON, name string + exactName bool + resCount uint + ) for rows.Next() { - if foundResult { - return nil, fmt.Errorf("more than one result for pod %q: %w", idOrName, define.ErrCtrExists) - } - - if err := rows.Scan(&rawJSON); err != nil { + if err := rows.Scan(&rawJSON, &name); err != nil { return nil, fmt.Errorf("error retrieving pod %q ID from database: %w", idOrName, err) } - foundResult = true + if name == idOrName { + exactName = true + break + } + resCount++ } - if !foundResult { - return nil, fmt.Errorf("no pod with name or ID %s found: %w", idOrName, define.ErrNoSuchPod) + if !exactName { + if resCount == 0 { + return nil, fmt.Errorf("no pod with name or ID %s found: %w", idOrName, define.ErrNoSuchPod) + } else if resCount > 1 { + return nil, fmt.Errorf("more than one result for pod %q: %w", idOrName, define.ErrCtrExists) + } } return s.createPod(rawJSON) diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index b636c23ea3..b490e9931b 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -718,4 +718,21 @@ var _ = Describe("Podman create", func() { setup.WaitWithDefaultTimeout() Expect(setup).Should(Exit(0)) }) + + It("create container with name subset of existing ID", func() { + create1 := podmanTest.Podman([]string{"create", "-t", ALPINE, "top"}) + create1.WaitWithDefaultTimeout() + Expect(create1).Should(Exit(0)) + ctr1ID := create1.OutputToString() + + ctr2Name := ctr1ID[:5] + create2 := podmanTest.Podman([]string{"create", "-t", "--name", ctr2Name, ALPINE, "top"}) + create2.WaitWithDefaultTimeout() + Expect(create2).Should(Exit(0)) + + inspect := podmanTest.Podman([]string{"inspect", "--format", "{{.Name}}", ctr2Name}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).Should(Equal(ctr2Name)) + }) }) diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 0b1551d138..35534b5d9a 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -1199,4 +1199,20 @@ ENTRYPOINT ["sleep","99999"] Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) + It("create pod with name subset of existing ID", func() { + create1 := podmanTest.Podman([]string{"pod", "create"}) + create1.WaitWithDefaultTimeout() + Expect(create1).Should(Exit(0)) + pod1ID := create1.OutputToString() + + pod2Name := pod1ID[:5] + create2 := podmanTest.Podman([]string{"pod", "create", pod2Name}) + create2.WaitWithDefaultTimeout() + Expect(create2).Should(Exit(0)) + + inspect := podmanTest.Podman([]string{"pod", "inspect", "--format", "{{.Name}}", pod2Name}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).Should(Equal(pod2Name)) + }) })