Skip to content

Conversation

@hoelter
Copy link

@hoelter hoelter commented Jan 5, 2025

The "backup-unset-public-key-encryption" is not currently functioning. These changes resolve that.

I updated the set / unset functions to enable removing either the public-key-encryption or the encryption-passphrase in isolation without removing the other and added some notes to the readme.

local SERVICE_ROOT="${PLUGIN_DATA_ROOT}/${SERVICE}"
local SERVICE_BACKUP_ENCRYPTION_ROOT="${SERVICE_ROOT}/backup-encryption/"

mkdir "$SERVICE_BACKUP_ENCRYPTION_ROOT"
Copy link
Author

Choose a reason for hiding this comment

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

Added '-p' flag to prevent this command from failing if the encryption directory already exists. This will occur after an encryption type was set and then removed.

rm "$SERVICE_BACKUP_ENCRYPTION_ROOT/ENCRYPTION_KEY"
}

service_backup_unset_encryption() {
Copy link
Author

Choose a reason for hiding this comment

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

This was a duplicate of the above function prior to this change.

[[ -z "$SERVICE" ]] && dokku_log_fail "Please specify a valid name for the service"
verify_service_name "$SERVICE"
service_backup_unset_public_key_encryption "$SERVICE" # TODO: [22.03.2024 by Mykola]
service_backup_unset_public_key_encryption "$SERVICE"
Copy link
Author

Choose a reason for hiding this comment

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

This wasn't pointing to an actual function and would blow up at this point prior to the above change in common-functions.

[[ ${argv[0]} == "$cmd" ]] && shift 1
declare SERVICE="$1"
is_implemented_command "$cmd" || dokku_log_fail "Not yet implemented" # TODO: [22.03.2024 by Mykola]
is_implemented_command "$cmd" || dokku_log_fail "Not yet implemented"
Copy link
Author

Choose a reason for hiding this comment

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

Cleaned up todos

@hoelter hoelter changed the title Fixed the backup-unset-public-key-encryption command. fix: backup-unset-public-key-encryption command Mar 1, 2025
@hoelter
Copy link
Author

hoelter commented Aug 12, 2025

Anything I can do to help get this merged?

README.md Outdated
Comment on lines 649 to 652
If both passphrase and public key forms of encryption are set, the public key encryption will take precedence.

The underlying core backup script is present [here](https://github.com/dokku/docker-s3backup/blob/main/backup.sh).

Copy link
Member

Choose a reason for hiding this comment

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

The readme is autogenerated, so you'll need to update the doc generation to get this into place as it will be wiped on next generation.

You can use the bin/generate script to regenerate the readme to test your changes.

Copy link
Author

Choose a reason for hiding this comment

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

I updated the generation script and moved a couple of the comments into the description variables so they'd appear with the specific method info. Thee desc lines are a bit longer now, so let me know if you'd like me to make any stylistic changes or shift the location of the info -- maybe putting it all at the top level where I made the generation script edits.

@hoelter hoelter force-pushed the public-key-encryption-unset-fix branch from dbf99bf to c9f2e6e Compare August 13, 2025 02:28
@hoelter
Copy link
Author

hoelter commented Nov 19, 2025

@josegonzalez anything else I can do to help get this merged upstream?

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