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

overlay/15fcos: Print information regarding an ignition config and authorized keys #344

Merged
merged 1 commit into from
May 22, 2020
Merged

overlay/15fcos: Print information regarding an ignition config and authorized keys #344

merged 1 commit into from
May 22, 2020

Conversation

sohankunkerkar
Copy link
Member

Fixes #279
Note:
As per the resolution provided in the above issue(coreos/fedora-coreos-tracker#279 (comment)),

@sohankunkerkar sohankunkerkar changed the title WIP: Add more information in the serial console log if no Ignition is provided WIP: Add more information to the serial console log if no Ignition is provided Apr 14, 2020
cgwalters
cgwalters reviewed Apr 14, 2020
View reviewed changes
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I am uncomfortable with poking into all of /home for the reasons below. I think it'd be safer instead if we changed afterburn and Ignition to write out "stamp files" like:
/var/lib/ignition/ssh-keys-written
/run/afterburn/ssh-keys-written

or so. Then all this service needs to do is run test -f on those files, and it doesn't need any special privileges.

The downside of this approach is it requires patching those projects. For the Ignition case we discussed caching the config somewhere; if we did that we could just parse the config.

Maybe it's simplest to start with patching afterburn to write such a stamp file.

overlay.d/15fcos/usr/lib/systemd/system/coreos-check-ssh-keys.service Outdated Show resolved Hide resolved
overlay.d/15fcos/usr/libexec/ssh_authorized_keys.sh Outdated Show resolved Hide resolved
@cgwalters
Copy link
Member

Or, building on the idea from coreos/ignition#958 instead of the "stamp files" we could use a journal entry.

cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request Apr 14, 2020
@cgwalters
systemd: Toggle on ProtectHome=true
33498d1 
We might as well make use of this.  I thought of this while
reviewing coreos/fedora-coreos-config#344
@cgwalters cgwalters mentioned this pull request Apr 14, 2020
Closed
@sohankunkerkar sohankunkerkar marked this pull request as draft April 15, 2020 18:53
This was referenced Apr 15, 2020
Merged
Closed
Closed
Merged
@sohankunkerkar
Copy link
Member Author

It looks like we need to cut a new release of ignition and afterburn repo to proceed with the CI testing. This PR is dependent on the following PRs which are merged recently.

coreos/ignition#958
coreos/ignition#964
coreos/afterburn#397

@cgwalters
Copy link
Member

It looks like we need to cut a new release of ignition and afterburn repo to proceed with the CI testing. This PR is dependent on the following PRs which are merged recently.

Yeah, that's currently a manual process.

@rfairley rfairley mentioned this pull request May 8, 2020
Merged
@sohankunkerkar sohankunkerkar changed the title WIP: Add more information to the serial console log if no Ignition is provided overlay/15fcos: Print information regarding an ignition config and authorized keys May 8, 2020
@sohankunkerkar sohankunkerkar marked this pull request as ready for review May 8, 2020 22:49
dustymabe
dustymabe approved these changes May 22, 2020
View reviewed changes
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM - this was a long time coming. Nice work getting changes into all relevant upstreams and then bringing it all together here.

@dustymabe
Copy link
Member

when I test this using cosa run I don't see the output for the ssh keys check the issue.d file is empty:

/run/console-login-helper-messages/issue.d/30_ssh_authorized_keys.issue

@dustymabe
Copy link
Member

when I test this using cosa run I don't see the output for the ssh keys check the issue.d file is empty:

/run/console-login-helper-messages/issue.d/30_ssh_authorized_keys.issue

turned out to be https://github.com/coreos/fedora-coreos-config/pull/344/files#r429340945

@rfairley rfairley mentioned this pull request May 22, 2020
Closed
@sohankunkerkar
overlay/15fcos: Print info regarding an ignition config and ssh-autho…
b3c898f 
…rized keys

This PR addresses the concern raised in coreos/fedora-coreos-tracker#279
which talks about systems behavior when no igntion is provided. Currently, we're tracking ignitionConfig
messages(coreos/fedora-coreos-tracker#279) and ssh-authorized keys info
(coreos/afterburn#397) by sending the structured entry into journald log. Here,
the systemd units are written to scrape through that information to display meaningful data to users.
@dustymabe
Copy link
Member

ok all problems have been addressed!

@dustymabe dustymabe merged commit 4f23b23 into coreos:testing-devel May 22, 2020
@dustymabe dustymabe mentioned this pull request May 22, 2020
Closed
@dustymabe
Copy link
Member

and on GCP I now see:

gcp

Nice work Sohan! Thanks everyone for all the reviews!

bgilbert
bgilbert reviewed Jun 4, 2020
View reviewed changes
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Couple spelling nits, FWIW.

overlay.d/15fcos/usr/lib/systemd/system/coreos-check-ignition-config.service Show resolved Hide resolved
overlay.d/15fcos/usr/lib/systemd/system/coreos-check-ssh-keys.service Show resolved Hide resolved
@sohankunkerkar
Copy link
Member Author

Couple spelling nits, FWIW.

My bad. Thanks for point this out. I will create a follow-up PR to fix it.

@sohankunkerkar sohankunkerkar mentioned this pull request Jun 5, 2020
Merged
sohankunkerkar added a commit that referenced this pull request Jun 5, 2020
@sohankunkerkar
overlay/15fcos: few nits
b0b2dbf 
Follow-up to #344 (review)
@dustymabe dustymabe mentioned this pull request Jun 8, 2020
Closed
@sohankunkerkar sohankunkerkar mentioned this pull request Jul 16, 2020
Merged
@sohankunkerkar sohankunkerkar mentioned this pull request Aug 7, 2020
Merged
@sohankunkerkar sohankunkerkar deleted the fix-ssh-keys-info branch September 21, 2020 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfairley rfairley rfairley approved these changes

@dustymabe dustymabe dustymabe approved these changes

@bgilbert bgilbert bgilbert left review comments

@lucab lucab lucab approved these changes

@cgwalters cgwalters Awaiting requested review from cgwalters

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sohankunkerkar @cgwalters @dustymabe @lucab @bgilbert @rfairley