Skip to content

Conversation

@hoelter
Copy link
Contributor

@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
Contributor 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
Contributor 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
Contributor 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
Contributor 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
Contributor Author

hoelter commented Aug 12, 2025

Anything I can do to help get this merged?

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
Contributor 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.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the longer descriptions should either be part of an example or just in errata that gets injected into the generated docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I shortened the descriptions and moved the second info part to a #E line

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

hoelter commented Nov 19, 2025

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

@josegonzalez josegonzalez merged commit b805936 into dokku:master Dec 3, 2025
2 checks passed
josegonzalez added a commit to dokku/dokku-clickhouse that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-couchdb that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-elasticsearch that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-graphite that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-mariadb that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-meilisearch that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-memcached that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-mongo that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-mysql that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-omnisci that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-pushpin that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-rabbitmq that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-redis that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-rethinkdb that referenced this pull request Dec 3, 2025
josegonzalez added a commit to dokku/dokku-typesense that referenced this pull request Dec 3, 2025
This was referenced Dec 3, 2025
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