Skip to content
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.

Sign up for GitHub

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

Jump to bottom

properly load inherited foreman settings #419

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+43 −18
Open

properly load inherited foreman settings #419

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c2679c
properly load inherited foreman settings
evgeni Sep 2, 2024
7336048
Remove PASS_NAME_{GPG,KEY} from main settings file
ekohl Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions settings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ load_settings() {
# Expected: FULLGPGKEY
# Optional: FULLVERSION
. "$RELEASEDIR/settings"

if [[ $PROJECT != foreman ]]; then
ekohl marked this conversation as resolved.
Show resolved Hide resolved
if [[ -z "$FOREMAN_VERSION" ]]; then
echo "please set FOREMAN_VERSION in $RELEASEDIR/settings"
exit 1
elif [[ "$FOREMAN_VERSION" != "none" ]]; then
# load foreman settings, which will contain keys etc
. "releases/foreman/${FOREMAN_VERSION}/settings"
Copy link
Member Author

@evgeni evgeni Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

mhh, no, this overwrites OSes that have been set in client/(version/)?settings before. damn.

fi
else
FOREMAN_VERSION="$VERSION"
fi
}

# Prepare settings file
Expand All @@ -29,19 +41,6 @@ if [[ ! -d "$RELEASEDIR" ]] ; then
echo "FULLVERSION='${VERSION}.0-rc1'" > "$RELEASEDIR/settings"
fi

if [[ $PROJECT != foreman ]]; then
load_settings
if [[ -z "$FOREMAN_VERSION" ]]; then
echo "please set FOREMAN_VERSION in $RELEASEDIR/settings"
exit 1
elif [[ "$FOREMAN_VERSION" != "none" ]]; then
# load foreman settings, which will contain keys etc
. "releases/foreman/${FOREMAN_VERSION}/settings"
fi
else
FOREMAN_VERSION="$VERSION"
fi

SIGNER="[email protected]"
SIGNER_NAME="Foreman Automatic Signing Key"
OSES=""
Expand All @@ -52,8 +51,6 @@ TARDIR="$RELEASEDIR/tarballs"
EXTRASDIR="$RELEASEDIR/extras"
HOSTS="web01.osuosl.theforeman.org,"
DOCROOT="/var/www/vhosts/downloads/htdocs"
PASS_NAME_GPG="theforeman/releases/foreman/$FOREMAN_VERSION-gpg"
PASS_NAME_KEY="theforeman/releases/foreman/$FOREMAN_VERSION-key"
XARGS_JOBS="-n 20 -P 4"
GIT_DIR="${GIT_DIR:-$HOME/dev}"
GIT_USE_WORKTREES=false
Expand All @@ -70,6 +67,8 @@ GPG_EXPIRE="1y"

load_settings

PASS_NAME_GPG="theforeman/releases/foreman/$FOREMAN_VERSION-gpg"
PASS_NAME_KEY="theforeman/releases/foreman/$FOREMAN_VERSION-key"
Copy link
Member

Choose a reason for hiding this comment

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

This overrides the settings, which means you can't override it anymore as we do:

releases/pulpcore/settings
2:PASS_NAME_GPG="theforeman/releases/pulpcore/$VERSION-gpg"
3:PASS_NAME_KEY="theforeman/releases/pulpcore/$VERSION-key"

releases/foreman-debian/settings
1:PASS_NAME_GPG="theforeman/releases/foreman/debian-$VERSION-gpg"
2:PASS_NAME_KEY="theforeman/releases/foreman/debian-$VERSION-key"

releases/candlepin/settings
1:PASS_NAME_GPG="theforeman/releases/candlepin/$VERSION-gpg"
2:PASS_NAME_KEY="theforeman/releases/candlepin/$VERSION-key"

It's also not like we can use $PROJECT like this:

Suggested change
PASS_NAME_GPG="theforeman/releases/foreman/$FOREMAN_VERSION-gpg"
PASS_NAME_KEY="theforeman/releases/foreman/$FOREMAN_VERSION-key"
PASS_NAME_GPG="theforeman/releases/$PROJECT/$VERSION-gpg"
PASS_NAME_KEY="theforeman/releases/$PROJECT/$VERSION-key"

What I'm leaning to is move these definitions into releases/foreman/settings (and not have any default) but I don't think we load that now when inheriting settings.

Alternatively you can use:

Suggested change
PASS_NAME_GPG="theforeman/releases/foreman/$FOREMAN_VERSION-gpg"
PASS_NAME_KEY="theforeman/releases/foreman/$FOREMAN_VERSION-key"
PASS_NAME_GPG="${PASS_NAME_GPG:-theforeman/releases/foreman/$FOREMAN_VERSION-gpg}"
PASS_NAME_KEY="${PASS_NAME_KEY:-theforeman/releases/foreman/$FOREMAN_VERSION-key}"

Copy link
Member Author

Choose a reason for hiding this comment

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

dang.

so the problem I was trying to fix is:
When signing Katello, $OSES ends up '' as it first loaded katello/4.14/settings, then foreman/3.12/settings and then overwrote OSES with the value in settings.
To fix that, I moved the "load inherited foreman stuff" logic into load_settings, but that that meant that FOREMAN_VERSION is not yet populated when the old location of PASS_NAME_* was used, so I moved that too.

Copy link
Member

Choose a reason for hiding this comment

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

I added a commit that I think addresses the concerns. It's not pretty, but I think it'll work.

HALFGPGKEY="$(echo ${FULLGPGKEY: -16} | tr '[A-Z]' '[a-z]')"

show_gpg_password() {
Expand Down
Loading