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

Drop GSS proxy privilege separation from direct AD integration proc (3.10) #3325

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

asteflova
Copy link
Member

What changes are you introducing?

Applying the changes implemented in #3149 to versions 3.10 and earlier.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Explained in #3149

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

N/A

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

* [ ] Foreman 3.12/Katello 4.14 (Satellite 6.16)
* [ ] Foreman 3.11/Katello 4.13

  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

@asteflova asteflova added the Needs style review Requires a review from docs style/grammar perspective label Sep 27, 2024
Copy link

@asteflova
Copy link
Member Author

asteflova commented Oct 1, 2024

This is just a cherry-pick of just re-applies changes approved elsewhere (see the description) > tech review done.

@asteflova asteflova added the tech review done No issues from the technical perspective label Oct 1, 2024
@maximiliankolb
Copy link
Contributor

@asteflova Please use -x when cherry-picking commit so that we can see the original commit in the commit message. In which files did you resolve merge conflicts?

@asteflova
Copy link
Member Author

Sorry @maximiliankolb poor choice of words in my comment above. This PR doesn't cherry-pick the commit from 3.11 and above because of all the other changes that have been happening in external authentication since 3.11. We are essentially dealing with different documentation sections. What I should have wrote is that I updated the relevant parts of 3.10 docs to match the corresponding parts of 3.11and above. The goal was to remove references to GSS proxy on 3.10 and lower.

Would you prefer me to do a standard cherry-pick? I could do that but then I'd have to apply another commit that would revert some of those changes back to the original state so that the updated sections are consistent with the rest of 3.10 (mostly, this would mean changes to introductions and assembly includes).

Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

@maximiliankolb
Copy link
Contributor

Would you prefer me to do a standard cherry-pick?

I was just being lazy and trying to review as little as possible. A cherry-pick "by hand" is also OK here; maybe you can edit your commit message before merging and reference the PR/original commit in some way.

@maximiliankolb maximiliankolb added style review done No issues from docs style/grammar perspective and removed Needs style review Requires a review from docs style/grammar perspective labels Oct 1, 2024
@asteflova
Copy link
Member Author

I was just being lazy

Nah, I was the lazy one and didn't want to deal with all the conflicts and manual reverts. Thanks for the ack.

maybe you can edit your commit message before merging and reference the PR/original commit in some way.

Will do, thanks for the tip.

@asteflova asteflova merged commit 9fdcce8 into theforeman:3.10 Oct 1, 2024
7 of 8 checks passed
asteflova added a commit that referenced this pull request Oct 1, 2024
asteflova added a commit that referenced this pull request Oct 1, 2024
asteflova added a commit that referenced this pull request Oct 1, 2024
asteflova added a commit that referenced this pull request Oct 1, 2024
asteflova added a commit that referenced this pull request Oct 1, 2024
@asteflova
Copy link
Member Author

Merged to "master" and cherry-picked:

ee9d962..884d19b 3.9 -> 3.9
8f8e5ea..3519b53 3.8 -> 3.8
b584901..944e3b3 3.7 -> 3.7
a960379..ce3466d 3.6 -> 3.6
e9460ea..ecf8d07 3.5 -> 3.5

Minor conflicts, for example involving the dropped {package-install-project} attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style review done No issues from docs style/grammar perspective tech review done No issues from the technical perspective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants