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

fb_apt: Significant updates #250

Closed
wants to merge 1 commit into from
Closed

fb_apt: Significant updates #250

wants to merge 1 commit into from

Conversation

jaymzh
Copy link
Collaborator

@jaymzh jaymzh commented Feb 3, 2025

  • Deprecate node['fb_apt']['repos'] which was always a bad API
    (sorry), and replace it with node['fb_apt']['sources'] which
    integrates nicely with the new node['fb_apt']['keymap']
  • Deprecate node['fb_apt']['keys'] which was very broken on modern
    apt and replace it with a new node['fb_apt']['keymap']
  • Update syntax for security and update repos on modern debian and
    ubuntu
  • Remove old Ubuntu 16 cruft
  • Lots of cleanups and refactoring for readability

Signed-off-by: Phil Dibowitz phil@ipom.com

@jaymzh jaymzh force-pushed the fb_apt_modernization branch from 248cd2b to ebc93be Compare February 3, 2025 10:23
jaymzh
jaymzh commented Feb 3, 2025
View reviewed changes
cookbooks/fb_apt/attributes/default.rb
}
# fb_apt must be defined for this to work...
keys = FB::Apt.get_official_keyids(node).map { |id| [id, nil] }.to_h
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is OK because we only look at keyis in /etc/trusted.gpg now, not all keyids in /etc/trusted.gpg.d/* - as those are cleaned up separately.

@jaymzh jaymzh requested a review from dafyddcrosby February 3, 2025 10:25
@jaymzh jaymzh force-pushed the fb_apt_modernization branch 2 times, most recently from 1fe7d2c to 274ae92 Compare February 6, 2025 20:10
@ericnorris
Copy link
Contributor

Hey @jaymzh, not sure if you saw this, but @adsr filed #241, and we ended up making an internal fork (called etsy_apt) that I think does some similar things as this PR. I'm going to look into seeing if we can share our version to see how much overlap there is - it'd be nice if we could use the same version!

@jaymzh
Copy link
Collaborator Author

jaymzh commented Feb 6, 2025

I had not seen it, no. But this should solve your problem without forking and in a backwards compat way

@jaymzh jaymzh force-pushed the fb_apt_modernization branch 2 times, most recently from df41c12 to c21c549 Compare February 8, 2025 19:00
@jaymzh jaymzh mentioned this pull request Feb 8, 2025
Closed
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 8, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
d27c586 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 8, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
10c6dee 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 8, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
932720d 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 8, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
ca57c44 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 8, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
7c0a3a5 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 13, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
3b34875 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@dafyddcrosby
Copy link
Contributor

Got minor cookstyle nits: http://github.com/facebook/chef-cookbooks/actions/runs/13218441920/job/36900538177?pr=250

@jaymzh jaymzh force-pushed the fb_apt_modernization branch from c21c549 to 5e0896f Compare February 13, 2025 20:45
@jaymzh
Copy link
Collaborator Author

jaymzh commented Feb 13, 2025

Got minor cookstyle nits: http://github.com/facebook/chef-cookbooks/actions/runs/13218441920/job/36900538177?pr=250

Sorry! Fixed.

@facebook-github-bot
Copy link
Contributor

@dafyddcrosby has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

davide125
davide125 requested changes Feb 13, 2025
View reviewed changes
Copy link
Member

@davide125 davide125 left a comment

Choose a reason for hiding this comment

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

nits from the internal cookstyle run

jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 13, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
cf6ea7a 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh
fb_apt: Significant updates
97c16fb 
* Deprecate `node['fb_apt']['repos']` which was always a bad API
  (sorry), and replace it with `node['fb_apt']['sources']` which
  integrates nicely with the new `node['fb_apt']['keymap']`
* Deprecate `node['fb_apt']['keys']` which was very broken on modern
  apt and replace it with a new `node['fb_apt']['keymap']`
* Update syntax for security and update repos on modern debian and
  ubuntu
* Remove old Ubuntu 16 cruft
* Lots of cleanups and refactoring for readability

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh jaymzh force-pushed the fb_apt_modernization branch from 5e0896f to 97c16fb Compare February 13, 2025 21:41
@facebook-github-bot
Copy link
Contributor

@jaymzh has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@dafyddcrosby has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a106940.

jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 13, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
1030af0 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/chef-cookbooks that referenced this pull request Feb 13, 2025
@jaymzh
CI fixes: yum repos; disable logind; chef 18
2ac2573 
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in facebook#250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>
facebook-github-bot pushed a commit that referenced this pull request Feb 14, 2025
@jaymzh @facebook-github-bot
CI fixes: yum repos; disable logind; chef 18 (#254)
1014511 
Summary:
* centos8 mirrors have moved, adjust kitchen setup accordingly.
* logind can't run in containers
* modern chef
* Newer chef requires at least debian 11 for SSL compatability
* rsyslog needs several tweaks to run in containers, added to ci_fixes
* default apache config in centos points to some certs, so make those

This makes everything green _except_ for debian which requires
significant refactors of fb_apt, which can be found in #250, but I
didn't want production-effecting stuff mixed with CI fixes

Signed-off-by: Phil Dibowitz <phil@ipom.com>

Pull Request resolved: #254

Differential Revision: D69275524

fbshipit-source-id: 6b6e7f0a10263d28e999f3adaa0b8c916459a604
@jaymzh
Copy link
Collaborator Author

jaymzh commented Feb 23, 2025

@ericnorris - just curious - did you end up looking at this at all? It'd be great if you didn't have to maintain a fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davide125 davide125 davide125 requested changes

@joshuamiller01 joshuamiller01 Awaiting requested review from joshuamiller01 joshuamiller01 is a code owner

@dafyddcrosby dafyddcrosby Awaiting requested review from dafyddcrosby

Assignees
No one assigned
Labels
CLA Signed Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jaymzh @ericnorris @dafyddcrosby @facebook-github-bot @davide125