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

check all headers, go version bump #171

Merged
merged 3 commits into from
May 11, 2022
Merged

Conversation

ibihim
Copy link
Collaborator

@ibihim ibihim commented Apr 29, 2022

What

  • set go-version on build workflow to 1.17.9
  • remove dead code: DeepCopy
  • check for all headers, not just the first entry, when creating AttributesRecords

Why

  • go version 1.17.8 and 1.17.9 have security fixes
  • unused code is bad code
  • don't give the impression that a second header is checked, even though it isn't

Signed-off-by: Krzysztof Ostrowski kostrows@redhat.com

@ibihim ibihim requested a review from s-urbaniak April 29, 2022 16:17
@s-urbaniak
Copy link
Collaborator

@ibihim generally, lgtm. Can you please:

  • separate the go bump in a separate commit
  • elaborate in the commit fixing the headers the following topics:
    • motivation
    • solution strategy

Currently it is not clear what this is fixing, why and how.

@ibihim
Copy link
Collaborator Author

ibihim commented May 9, 2022

yes, thank you.

@ibihim ibihim force-pushed the go-version-bump branch 2 times, most recently from 4652844 to d01d218 Compare May 9, 2022 12:16
ibihim added 3 commits May 9, 2022 14:24
Signed-off-by: Krzysztof Ostrowski <kostrows@redhat.com>
It is considered problematic, when only the first header value is
being authorized and the rest is dropped. It might give the false
impression that all given values are authorized.

Signed-off-by: Krzysztof Ostrowski <kostrows@redhat.com>
Signed-off-by: Krzysztof Ostrowski <kostrows@redhat.com>
@s-urbaniak
Copy link
Collaborator

lgtm

@s-urbaniak s-urbaniak merged commit af18a1d into brancz:master May 11, 2022
@ibihim ibihim deleted the go-version-bump branch June 29, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants