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.

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

Query target version in discovery script #39033

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

hugoShaka
Copy link
Contributor

@hugoShaka hugoShaka commented Mar 6, 2024

Unlike what's written in the file shebang, this is not bash but sh 🙃 .

I had to rewrite to bourne-shell compliant script.

I removed the $PACKAGE_LIST var entirely for 2 reasons:

  • this cannot be safely pbuilt, passed and parsed by apt. A naive implementation would allow arbitrarily package install from the version server. A smarter implementation would use bash arrys. I did this initially but SSM doesn't run us as bash.
  • the way to pin versions differs between apt/zypper and yum. One package list doesn't fit all distros.

The resulting script is a bit ugly.

changelog: Fix a bug when using automatic updates and the discovery service. The default install script now installs the correct teleport version by querying the version server.

@hugoShaka hugoShaka requested a review from fspmarshall March 6, 2024 18:23
@github-actions github-actions bot requested review from avatus and lxea March 6, 2024 18:23
Copy link

github-actions bot commented Mar 6, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

Copy link
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

I think this is a good quick fix. Long-run, lets add a subcommand to the upgrader to ask it to pull the target version, that way it can be the single source of truth for target version.

@hugoShaka hugoShaka force-pushed the hugo/fix-discovery-script-version branch from 7fc36ff to e71b33d Compare March 6, 2024 21:28
Copy link

github-actions bot commented Mar 6, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@hugoShaka hugoShaka requested a review from fspmarshall March 6, 2024 21:33
Copy link

github-actions bot commented Mar 6, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

1 similar comment
Copy link

github-actions bot commented Mar 6, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@hugoShaka
Copy link
Contributor Author

I think this is a good quick fix. Long-run, lets add a subcommand to the upgrader to ask it to pull the target version, that way it can be the single source of truth for target version.

I agree, I think what roman suggested (having a subcommand in the teleport-upgrader package that does the install) sounds like a good idea.

The install flow would be

  • install the updater
  • run teleport-upgrade install --proxy=foo.teleport.sh
  • the updater runs the apt/um/whatever logic to install the package

Copy link
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

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

It seems that there's a typo in TELEPORT_UPDATER_PACKAGE var.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that it also installs teleport and the updater script, we must change the agentless script as well.

api/types/installers/installer.sh.tmpl Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 7, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

Copy link
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

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

We must change the agentless script as well

Copy link

github-actions bot commented Mar 7, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@hugoShaka
Copy link
Contributor Author

TIL, we install teleport for agentless, AND it updater. I'm not sure what advantage agentless discovery has over teleport at this point 🤷‍♂️

@marcoandredinis
Copy link
Contributor

TIL, we install teleport for agentless, AND it updater. I'm not sure what advantage agentless discovery has over teleport at this point 🤷‍♂️

It's only used for joining the cluster and configuring openssh.
It will also do cert rotations.

The true agentless mode is the EC2 ICE, but it only works for AWS EC2 and only a subset of distros.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 7, 2024
@hugoShaka hugoShaka added this pull request to the merge queue Mar 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 7, 2024
@hugoShaka hugoShaka enabled auto-merge March 7, 2024 20:25
@hugoShaka hugoShaka added this pull request to the merge queue Mar 7, 2024
Merged via the queue into master with commit c426d5b Mar 7, 2024
36 checks passed
@hugoShaka hugoShaka deleted the hugo/fix-discovery-script-version branch March 7, 2024 20:50
@public-teleport-github-review-bot

@hugoShaka See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Create PR
branch/v15 Create PR

hugoShaka added a commit that referenced this pull request Mar 7, 2024
* Query target version in discovery script

* Update api/types/installers/installer.sh.tmpl

Co-authored-by: Marco André Dinis <[email protected]>

* address marco's feedback

* fix tests

---------

Co-authored-by: Marco André Dinis <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Mar 8, 2024
* Query target version in discovery script

* Update api/types/installers/installer.sh.tmpl



* address marco's feedback

* fix tests

---------

Co-authored-by: Marco André Dinis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants