Skip to content

Changes to make DamCTF work #53

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

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Changes to make DamCTF work #53

wants to merge 28 commits into from

Conversation

detjensrobert
Copy link
Contributor

@detjensrobert detjensrobert commented May 6, 2025

Changes during DamCTF crunch time to get things working.

Notable changes:

This consolidates (almost) all of the error display handling into the cli
main(), which is idiomatic since the library side of this shouldn't need to
worry about how its errors are presented.

There are a couple spots where the Error case is a Vec of things, which can't
be handled by the single error handler in main. These have been changed to
do their custom error printing, then return a new error to main(). I added
TODO comments to these.

Signed-off-by: Robert Detjens <[email protected]>
Signed-off-by: Robert Detjens <[email protected]>
Signed-off-by: Robert Detjens <[email protected]>
@detjensrobert detjensrobert self-assigned this May 6, 2025
Signed-off-by: Robert Detjens <[email protected]>
Pull secrets are namespace-scoped, so need to be present in all of our
namespaces that use our images. Also update deployments to use these pull
secrets.

Signed-off-by: Robert Detjens <[email protected]>
We're reusing the same tags, so always pull latest so not stuck on stale image

Signed-off-by: Robert Detjens <[email protected]>
Signed-off-by: Robert Detjens <[email protected]>
@detjensrobert detjensrobert marked this pull request as ready for review May 13, 2025 03:41
@detjensrobert detjensrobert requested a review from KekoaM May 16, 2025 04:48
Copy link

Choose a reason for hiding this comment

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

future consideration: do we want a way to use non-letsencrypt ACME?

Copy link

@KekoaM KekoaM left a comment

Choose a reason for hiding this comment

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

Overall looks good
already tested in prod

@@ -48,6 +43,93 @@ pub async fn docker() -> Result<&'static bollard::Docker> {
}
}

/// Fetch registry login credentials from ~/.docker/config.json or $DOCKER_CONFIG
///
/// For now, this is only `docker.io` credentials, as it is the only registry
Copy link

Choose a reason for hiding this comment

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

Probably want to track this in an issue (perhaps separate from #59)

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.

use docker credentials during build Note for later: challenge pods now accept architecture: Note for later: now using official external-dns chart
2 participants