Commit Graph

169 Commits

Author SHA1 Message Date
Matt Heon 174631f726 Convert SpecGen values to be nullable where possible
SpecGen is our primary container creation abstraction, and is
used to connect our CLI to the Libpod container creation backend.
Because container creation has a million options (I exaggerate
only slightly), the struct is composed of several other structs,
many of which are quite large.

The core problem is that SpecGen is also an API type - it's used
in remote Podman. There, we have a client and a server, and we
want to respect the server's containers.conf. But how do we tell
what parts of SpecGen were set by the client explicitly, and what
parts were not? If we're not using nullable values, an explicit
empty string and a value never being set are identical - and we
can't tell if it's safe to grab a default from the server's
containers.conf.

Fortunately, we only really need to do this for booleans. An
empty string is sufficient to tell us that a string was unset
(even if the user explicitly gave us an empty string for an
option, filling in a default from the config file is acceptable).
This makes things a lot simpler. My initial attempt at this
changed everything, including strings, and it was far larger and
more painful.

Also, begin the first steps of removing all uses of
containers.conf defaults from client-side. Two are gone entirely,
the rest are marked as remove-when-possible.

[NO NEW TESTS NEEDED] This is just a refactor.

Signed-off-by: Matt Heon <mheon@redhat.com>
2024-01-30 10:42:24 -05:00
Oleksandr Redko 8bdf77aa20 Refactor: replace StringInSlice with slices.Contains
Signed-off-by: Oleksandr Redko <Oleksandr_Redko@epam.com>
2024-01-05 16:25:56 +02:00
Ed Santiago 047da19b5f (Temporary) Emergency CI fix: quay search is broken
Someone please revert this once quay search is fixed.

Signed-off-by: Ed Santiago <santiago@redhat.com>
2023-11-13 06:52:10 -07:00
Brent Baude 78798cab00 Automatic code cleanups - JetBrains
A bunch of cleanups as suggested by linters/etc in JetBrains IDE.

Signed-off-by: Brent Baude <bbaude@redhat.com>
2023-11-07 14:05:15 -06:00
Urvashi Mohnani ebe01ca292 Add e2e tests for farm build
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
2023-10-24 17:32:47 -04:00
Paul Holzinger 597ebeb60f
top: do not depend on ps(1) in container
This ended up more complicated then expected. Lets start first with the
problem to show why I am doing this:

Currently we simply execute ps(1) in the container. This has some
drawbacks. First, obviously you need to have ps(1) in the container
image. That is no always the case especially in small images. Second,
even if you do it will often be only busybox's ps which supports far
less options.

Now we also have psgo which is used by default but that only supports a
small subset of ps(1) options. Implementing all options there is way to
much work.

Docker on the other hand executes ps(1) directly on the host and tries
to filter pids with `-q` an option which is not supported by busybox's
ps and conflicts with other ps(1) arguments. That means they fall back
to full ps(1) on the host and then filter based on the pid in the
output. This is kinda ugly and fails short because users can modify the
ps output and it may not even include the pid in the output which causes
an error.

So every solution has a different drawback, but what if we can combine
them somehow?! This commit tries exactly that.

We use ps(1) from the host and execute that in the container's pid
namespace.
There are some security concerns that must be addressed:
- mount the executable paths for ps and podman itself readonly to
  prevent the container from overwriting it via /proc/self/exe.
- set NO_NEW_PRIVS, SET_DUMPABLE and PDEATHSIG
- close all non std fds to prevent leaking files in that the caller had
  open
- unset all environment variables to not leak any into the contianer

Technically this could be a breaking change if somebody does not
have ps on the host and only in the container but I find that very
unlikely, we still have the exec in container fallback.

Because this can be insecure when the contianer has CAP_SYS_PTRACE we
still only use the podman exec version in that case.

This updates the docs accordingly, note that podman pod top never falls
back to executing ps in the container as this makes no sense with
multiple containers so I fixed the docs there as well.

Fixes #19001
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2215572

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2023-07-10 13:32:55 +02:00
Paul Holzinger 6eaf8a271d
tests: fix "Storing signatures" check
After[1] c/image no longer prints "Storing signatures" so we should
not check for it.

[1] https://github.com/containers/image/pull/2001

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2023-06-27 18:04:42 +02:00
Ed Santiago 6a696cb8fd podman-registry: simpler, safer invocations
First: fix podman-registry script so it preserves the initial $PODMAN,
so all subsequent invocations of ps, logs, and stop will use the
same binary and arguments. Until now we've handled this by requiring
that our caller manage $PODMAN (and keep it the same), but that's
just wrong.

Next, simplify the golang interface: move the $PODMAN setting into
registry.go, instead of requiring e2e callers to set it. (This
could use some work: the local/remote conditional is icky).

IMPORTANT: To prevent registry.go from using the wrong podman binary,
the Start() call is gone. Only StartWithOptions() is valid now.

And, minor cleanup: comments, and add an actual error-message check

Reason for this PR is a recurring flake, #18355, whose multiple
failure modes I truly can't understand. I don't think this PR
is going to fix it, but this is still necessary work.

Signed-off-by: Ed Santiago <santiago@redhat.com>
2023-06-07 12:16:18 -06:00
Erik Sjölund 685c736185 source code comments and docs: fix typos, language, Markdown layout
- fix a/an before noun
- fix loose -> lose
- fix "the the"
- fix lets -> let's
- fix Markdown layout
- fix a few typos
- remove unnecessary text in troubleshooting.md

Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
2023-05-22 07:52:16 +02:00
Paul Holzinger 19aabf440e
remote: exec inspect update exec session status
The remote API will wait 300s by default before conmon will call the
cleanup. In the meantime when you inspect an exec session started with
ExecStart() (so not attached) and it did exit we do not know that. If
a caller inspects it they think it is still running. To prevent this we
should sync the session based on the exec pid and update the state
accordingly.

For a reproducer see the test in this commit or the issue.

Fixes #18424

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2023-05-03 14:54:00 +02:00
Paul Holzinger c564d9d7af
ginkgo v2: remove CurrentGinkgoTestDescription()
This function is deprecated and replaced with CurrentSpecReport().
Also fix inconsitent callers.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2023-05-02 11:27:36 +02:00
Paul Holzinger 445815036f
update to ginkgo v2
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2023-05-02 11:27:35 +02:00
Ed Santiago 5f5bea0d12 bindings tests: bail out early on image errors
If Pull() fails, which it does on registry or network flakes,
bail out early: there's no point in continuing. Same with
Save() and restoreImageFromCache(), although those are
unlikely to fail.

Possibly better solution: retry with backoff. Left as exercise
for future maintainer.

Use Expect() for failure checks, and correct two existing
instances of Printf()/Exit() to also use Expect().

Signed-off-by: Ed Santiago <santiago@redhat.com>
2023-04-19 08:16:45 -06:00
Toshiki Sonoda 82f2f8258c Add search --cert-dir, --creds
When we searching any image at a container registry,
--cert-dir and --creds could be required
as well as push, pull, etc.

Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
2023-03-15 15:01:26 +09:00
Ygal Blum bddd3f5b5f Network Create: Add --ignore flag to support idempotent script
Add --ignore flag to the command line
Add a new parameter to the NetworkCreate interface in pkg/domain for CreateOptions
Add a new API Network CreateWithOptions in pkg/bindings
Remote API - Add a query parameter to set the ignore flag
Kube - use the IgnoreIfExists flag when creating the default network instead of handling the failure
Add e2e tests
Update man page for podman-network-create

Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
2022-12-15 10:58:54 +02:00
Paul Holzinger 2ddf1c5cbd
ginkgo tests: apply ginkgolinter fixes
I found the ginkgolinter[1] by accident, this looks for not optimal
matching and suggest how to do it better.

Overall these fixes seem to be all correct and they will give much
better error messages when something fails.
Check out the repo to see what the linter reports.

[1] https://github.com/nunnatsa/ginkgolinter

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-11-25 14:41:43 +01:00
Valentin Rothberg 44bac51fca bump golangci-lint to v1.49.0
Motivated to have a working `make lint` on Fedora 37 (beta).
Most changes come from the new `gofmt` standards.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
2022-10-17 09:19:41 +02:00
Chris Evich ba6f84666b
Fix a few missed io/ioutil -> os updates
Ref: https://github.com/containers/podman/pull/15871

Signed-off-by: Chris Evich <cevich@redhat.com>
2022-09-21 14:14:55 -04:00
Vladimir Kochnev cfdca82938
Mark some of the option fields as ignored in pkg/bindings
I realized that `params.Del("SkipTLSVerify")` doesn't have any
effect because keys are always lowercased. So it should really
be `params.Del("skiptlsverify")`.

There's also a little bug introduced by 3bf52aa and b1d1248: if
one passes `ProgressWriter` object having `Stringer` interface
i.e. `bytes.Buffer` it ends up been serialized in query with
`util.ToParams()`.

To circumvent both problems I propose to mark non-serializable
parameters with `schema:"-"` so there's no need to delete them from
resulting `url.Values`.

Signed-off-by: Vladimir Kochnev <hashtable@yandex.ru>
2022-08-26 09:55:17 +03:00
Vladimir Kochnev 3bf52aa338
Add ProgressWriter to PullOptions
Signed-off-by: Vladimir Kochnev <hashtable@yandex.ru>
2022-08-19 00:41:22 +03:00
Naoto Kobayashi b1d1248a18 pkg/bindings: Support writing image push progress to specified io.Writer
Currently bindings writes image push progress to os.Stderr.

Since os.Stderr is inconvenience for bindings caller to
process the progress messages, Added this support.

Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com>
2022-08-08 01:51:32 +09:00
Ed Santiago 94e0a364a8 Semiperiodoc cleanup of obsolete FIXMEs
Some refer to issues that are closed. Remove them.

Some are runc bugs that will never be fixed. Say so, and remove
the FIXME.

One (bps/iops) should probably be fixed. File an issue for it, and
update comment to include the issue# so my find-obsolete-skips script
can track it.

And one (rootless mount with a "kernel bug?" comment) is still
not fixed. Leave the skip, but add a comment documenting the symptom.

Signed-off-by: Ed Santiago <santiago@redhat.com>
2022-07-25 13:08:35 -06:00
Sascha Grunert a46f798831
pkg: switch to golang native error wrapping
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.

[NO NEW TESTS NEEDED]

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
2022-07-08 08:54:47 +02:00
Aditya R 5790caaef7
bindings: Add support for Delete in pkg/bingings/manifest
Bindings already support `Remove` which removes a manifest from the list
following function adds support for removing entire manifest for local
storage.

Similar functionality can be also used indirectly by using `Remove` defined in
image bindings

Signed-off-by: Aditya R <arajan@redhat.com>
2022-06-29 12:41:57 +05:30
Paul Holzinger 609b52f726
Bump version to v4.1.0-dev
I think we forgot to bump the version in the main branch. It should be
v4.1.0-dev now.
Also set the min api version to 4.0.0 as on the podman 4.0 branch.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-27 14:32:28 +02:00
Paul Holzinger 51fbf3da9e
enable gocritic linter
The linter ensures a common code style.
- use switch/case instead of else if
- use if instead of switch/case for single case statement
- add space between comment and text
- detect the use of defer with os.Exit()
- use short form var += "..." instead of var = var + "..."
- detect problems with append()
```
newSlice := append(orgSlice, val)
```
  This could lead to nasty bugs because the orgSlice will be changed in
  place if it has enough capacity too hold the new elements. Thus we
  newSlice might not be a copy.

Of course most of the changes are just cosmetic and do not cause any
logic errors but I think it is a good idea to enforce a common style.
This should help maintainability.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-26 18:12:22 +02:00
Paul Holzinger 5b4af0584d
replace golint with revive linter
golint, scopelint and interfacer are deprecated. golint is replaced by
revive. This linter is better because it will also check for our error
style: `error strings should not be capitalized or end with punctuation or a newline`

scopelint is replaced by exportloopref (already endabled)
interfacer has no replacement but I do not think this linter is
important.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-22 15:12:33 +02:00
Paul Holzinger 2a8e435671
enable staticcheck linter
Fix many problems reported by the staticcheck linter, including many
real bugs!

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-04-22 12:51:29 +02:00
Valentin Rothberg 06dd9136a2 fix a number of errcheck issues
Numerous issues remain, especially in tests/e2e.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
2022-03-22 13:15:28 +01:00
Valentin Rothberg 0f12b6fe55 linter: enable nilerr
A number of cases looked suspicious, so I marked them with `FIXME`s to
leave some breadcrumbs.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
2022-03-22 13:04:35 +01:00
Jhon Honce c4676c5719 Add version guard to libpod API endpoints
* Ensure meaningful behaviour when called with /v3.x.x semantics
* Change return code to 409 from 500 when client attempts to use an
  existing network name
* Update API bats test runner to support /v4.0.0 endpoints by default

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2022-02-09 14:22:42 -07:00
Daniel J Walsh 5736649eb8
Run codespell on code
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2022-01-21 09:52:12 -05:00
Paul Holzinger c0e0723463
rename --cni-config-dir to --network-config-dir
Since this option will also be used for netavark we should rename it to
something more generic. It is important that --cni-config-dir still
works otherwise we could break existing container cleanup commands.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-01-18 19:28:25 +01:00
Valentin Rothberg bd09b7aa79 bump go module to version 4
Automated for .go files via gomove [1]:
`gomove github.com/containers/podman/v3 github.com/containers/podman/v4`

Remaining files via vgrep [2]:
`vgrep github.com/containers/podman/v3`

[1] https://github.com/KSubedi/gomove
[2] https://github.com/vrothberg/vgrep

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
2022-01-18 12:47:07 +01:00
Jhon Honce 8a7e70919f Refactor manifest list operations
* Update method/function signatures use the manifest list name and
  images associated with the operation explicitly, in general

  func f(ctx context.Context, manifestListName string,
         ImageNames []string, options *fOptions)

* Leverage gorilla/mux Subrouters to support API v3.x and v4.x for
  manifests
* Make manifest API endpoints more RESTful
* Add PUT /manifest/{id} to update existing manifests
* Add manifests.Annotate to go bindings, uncommented unit test
* Add DELETE /manifest/{Id} to remove existing manifest list, use
  PUT /manifest/{id} to remove images from a list
* Deprecated POST /manifest/{id}/add and /manifest/{id}/remove, use
  PUT /manifest/{id} instead
* Corrected swagger godoc and updated to cover API changes
* Update podman manifest commands to use registry.Context()
* Expose utils.GetVar() to obtain query parameters by name
* Unexpose server.registerSwaggerHandlers, not sure why this was ever
  exposed.
* Refactored code to use http.Header instead of map[string]string when
  operating on HTTP headers.
* Add API-Version header support in bindings to allow calling explicate
  versions of the API. Header is _NOT_ forwarded to the API service.

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2022-01-14 16:13:35 -07:00
Paul Holzinger 495884b319
use libnetwork from c/common
The libpod/network packages were moved to c/common so that buildah can
use it as well. To prevent duplication use it in podman as well and
remove it from here.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2022-01-12 17:07:30 +01:00
Daniel J Walsh 8f2358eeaa
Add podman rm --depend
This option causes Podman to not only remove the specified containers
but all of the containers that depend on the specified
containers.
Fixes: https://github.com/containers/podman/issues/10360

Also ran codespell on the code

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2022-01-11 14:33:54 -05:00
Valentin Rothberg f241efb179 bindings rmi test: clarify behavior
The libpod/images/remove endpoint is not very REST-ish but, after some
debate, was decided to be implemented as for the following reasons.

First, it allows for batch removing images which improves performance
significantly.  Note that Docker does support `rmi -a`!

Second, it allows for hiding the logic of setting the right exit code to
use from the client and keep all the logic on the server.

Hence, when removing an image that does not exist, the server will
return a 200. The response, however, includes the error message to be
used *and* the exit code that podman-remote will use.

Fixes: #12441
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
2021-12-16 15:31:29 +01:00
Ed Santiago b63d696405 e2e tests: enable golint
...and fix problems found therewith.

Signed-off-by: Ed Santiago <santiago@redhat.com>
2021-11-29 08:30:00 -07:00
Chris Evich f3021f3f6e
Cirrus: Fix bindings test hang b/c logging config mismatch
Massive thanks to @edsantiago for tracking this down.

Ref: https://github.com/containers/podman/issues/12175

Signed-off-by: Chris Evich <cevich@redhat.com>
2021-11-18 13:50:11 -05:00
Moritz "WanzenBug" Wanzenböck 5df883e87d bindings: reuse context for API requests
One of the main uses of context.Context is to provide cancellation for
go-routines, including API requests. While all user-facing bindings
already used a context parameter, it was only used to pass the client
information around.

This commit changes the internal DoRequest wrapper to take an additional
context argument, and pass that to the http request. Previously, the context
was derived from context.Background(), which made it impossible to cancel
once started.

All the convenience wrappers already supported the context parameter, so the
only user facing change is that cancelling those context now works as one
would expect.

Signed-off-by: Moritz "WanzenBug" Wanzenböck <moritz@wanzenbug.xyz>
2021-11-15 15:42:39 +01:00
Paul Holzinger ba8eba83ef
Fix bindings container log test
The returned error was not checked, thus the test could hang forever
since it blocks on the log channel.

Also handle unexpectedEOF like EOF.

Fixes #12176

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2021-11-04 17:04:27 +01:00
Ondra Machacek f2115471dd Handle HTTP 409 error messages properly for Pod actions
This PR fixes the case when the API return HTTP 409 response. Where the
API return the body format different then for other HTTP error codes.

Signed-off-by: Ondra Machacek <omachace@redhat.com>
2021-11-02 12:28:52 +01:00
Jhon Honce 648882bf64 Add guard for BuildOptions.CommonBuildOpts
Existing images.Build() bindings code panicked when field was not
initialized.

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2021-09-30 14:30:06 -07:00
Paul Holzinger 85e8fbf7f3
Wire network interface into libpod
Make use of the new network interface in libpod.

This commit contains several breaking changes:
- podman network create only outputs the new network name and not file
  path.
- podman network ls shows the network driver instead of the cni version
  and plugins.
- podman network inspect outputs the new network struct and not the cni
  conflist.
- The bindings and libpod api endpoints have been changed to use the new
  network structure.

The container network status is stored in a new field in the state. The
status should be received with the new `c.getNetworkStatus`. This will
migrate the old status to the new format. Therefore old containers should
contine to work correctly in all cases even when network connect/
disconnect is used.

New features:
- podman network reload keeps the ip and mac for more than one network.
- podman container restore keeps the ip and mac for more than one
  network.
- The network create compat endpoint can now use more than one ipam
  config.

The man pages and the swagger doc are updated to reflect the latest
changes.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2021-09-15 20:00:20 +02:00
Jhon Honce d7256be807 Enhance bindings for IDE hints
* Follow https://pkg.go.dev/cmd/go#hdr-Generate_Go_files_by_processing_source
  for leading comment
* Add godoc strings for all exposed methods for IDE support
* Copy field godoc strings into generated code as function godoc string
* Remove unused/unnecessary fields from generator.go structures
* Cleanup code regarding template usage

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2021-09-14 07:47:24 -07:00
Daniel J Walsh 4fbc5b8fe7
Stop outputting 'healthy' on healthcheck
We should only print unhealthy if the check fails.  Currently this is
filling logs when users are running lots of healthchecks.

Improves: https://github.com/containers/podman/issues/11157

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2021-09-10 11:20:01 -04:00
cdoern d28e85741f InfraContainer Rework
InfraContainer should go through the same creation process as regular containers. This change was from the cmd level
down, involving new container CLI opts and specgen creating functions. What now happens is that both container and pod
cli options are populated in cmd and used to create a podSpecgen and a containerSpecgen. The process then goes as follows

FillOutSpecGen (infra) -> MapSpec (podOpts -> infraOpts) -> PodCreate -> MakePod -> createPodOptions -> NewPod -> CompleteSpec (infra) -> MakeContainer -> NewContainer -> newContainer -> AddInfra (to pod state)

Signed-off-by: cdoern <cdoern@redhat.com>
2021-08-26 16:05:16 -04:00
Jhon Honce 1dc6d14735 Fix file descriptor leaks and add test
* Add response.Body.Close() where needed to release HTTP
  connections to API server.
* Add tests to ensure no general leaks occur. 100% coverage would be
  required to ensure no leaks on any call.
* Update code comments to be godoc correct

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2021-08-24 16:36:10 -07:00
Valentin Rothberg 0f7d54b026 migrate Podman to containers/common/libimage
Migrate the Podman code base over to `common/libimage` which replaces
`libpod/image` and a lot of glue code entirely.

Note that I tried to leave bread crumbs for changed tests.

Miscellaneous changes:

 * Some errors yield different messages which required to alter some
   tests.

 * I fixed some pre-existing issues in the code.  Others were marked as
   `//TODO`s to prevent the PR from exploding.

 * The `NamesHistory` of an image is returned as is from the storage.
   Previously, we did some filtering which I think is undesirable.
   Instead we should return the data as stored in the storage.

 * Touched handlers use the ABI interfaces where possible.

 * Local image resolution: previously Podman would match "foo" on
   "myfoo".  This behaviour has been changed and Podman will now
   only match on repository boundaries such that "foo" would match
   "my/foo" but not "myfoo".  I consider the old behaviour to be a
   bug, at the very least an exotic corner case.

 * Futhermore, "foo:none" does *not* resolve to a local image "foo"
   without tag anymore.  It's a hill I am (almost) willing to die on.

 * `image prune` prints the IDs of pruned images.  Previously, in some
   cases, the names were printed instead.  The API clearly states ID,
   so we should stick to it.

 * Compat endpoint image removal with _force_ deletes the entire not
   only the specified tag.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
2021-05-05 11:30:12 +02:00