Skip to content

fix: recursively pull explicitly depended services regardless of active profile - #1449

Open
vkaylee wants to merge 3 commits into
containers:mainfrom
vkaylee:fix-resolve-profiles-dependencies
Open

fix: recursively pull explicitly depended services regardless of active profile#1449
vkaylee wants to merge 3 commits into
containers:mainfrom
vkaylee:fix-resolve-profiles-dependencies

Conversation

@vkaylee

@vkaylee vkaylee commented May 6, 2026

Copy link
Copy Markdown

Contributor Checklist:

Please make sure to read development guidelines in CONTRIBUTING.md. Pull requests that do not
follow the guidelines WILL TAKE LONGER TO REVIEW as the first review comment will be to follow
these guidelines.

If this PR adds a new feature that improves compatibility with docker-compose, please add a link
to the exact part of compose spec that the PR touches.

For any user-visible change please add a release note to newsfragments directory, e.g.
newsfragments/my_feature.feature. See newsfragments/README.txt for more details.

  • Added newsfragments/resolve_profiles_dependencies.bugfix

All changes require additional unit tests.

  • Added unit tests in tests/unit/test_resolve_profiles.py (all passing)

Description

Bug:
When using podman-compose --profile X up, if a service in profile X contains a depends_on block pointing to another service Y that is NOT part of profile X, podman-compose crashes with a KeyError: 'Y'. The filtering of profiles happened before dependency trees were validated, causing targeted dependencies to be stripped entirely from parsed compose data.

Fix:
This PR updates the _resolve_profiles method to recursively walk through explicitly defined dependencies (via depends_on and extends). If an active service targets an excluded service, that target service is now securely pulled back into the active execution graph regardless of its underlying profile structure.

This ensures proper fallback parity with docker-compose's Go implementation where "explicitly depended services are always started."

Testing:
Added comprehensive unit tests inside test_resolve_profiles.py which validate:

  • Explicit dict/list structure inclusions for depends_on.
  • Explicit dict/str structure inclusions for extends.
  • Multi-tier transitive dependencies (e.g. A -> B -> C).
  • Circular and non-existent edge-cases.

Comment thread podman_compose.py
elif isinstance(depends_on, list):
dep_names.extend(depends_on)

# Check extends

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No need for this commend and similar one above. Seems obvious from the next line.

"db": {"image": "postgres"},
}
services = self.compose._resolve_profiles(defined_services, {"test"})
self.assertIn("web", services)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Better use something like self.assertEqual(sorted(list(services.keys())), ["db", "web"]). This is both clearer and harder to get wrong, especially in cases when you'd otherwise use assertNotIn.

@p12tic p12tic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry for delay in review. I had several small comments. Otherwise PR looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants