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

Ignore case of MAC addresses and fix postgres log messages #1

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

synackd
Copy link
Collaborator

@synackd synackd commented Oct 31, 2023

Summary and Scope

Ignore case when dealing with MAC addresses. This is done by storing MAC addresses in lower case and converting any MAC addresses in requests to lower case before comparison. It is not desirable to distinguish MAC addresses as different based on case; therefore, we ignore it.

Also, some log message reformatting is present in this PR. For instance, there was a postgres.Add prefix in the Delete() function. Further, periods at the ends of log messages are removed so that it looks better when concatenated with other errors.

Issues and Related PRs

Migrated from bikeshack/hms-bss#7

Testing

Tested on:

  • Cable Guys SI Cluster (cg-head and compute nodes)
    • This is a system of x86-64 Gigabyte 272-Z32-00 machines running Rocky Linux 8.8

Added integration tests that test addition/retrieval/deletion using MAC addresses of differing case.

Risks and Mitigations

More comprehensive integration tests should be implemented in the future.

@synackd synackd self-assigned this Oct 31, 2023
To avoid having MAC addresses of different case being stored separately,
MAC addresses are stored in their lower-case form and any time MAC
address comparison occurs (e.g. for deletion/update/retrieval), the
passed MAC address is converted to lower case before the comparison is
done, effectively ignoring case.

This ensures that a difference in case does not result in duplicate MAC
addresses.
@synackd
Copy link
Collaborator Author

synackd commented Nov 20, 2023

Rebased onto new main with re-homing changes.

Copy link
Member

@alexlovelltroy alexlovelltroy left a comment

Choose a reason for hiding this comment

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

LGTM

@alexlovelltroy alexlovelltroy merged commit 49d50d3 into main Jan 19, 2024
0 of 6 checks passed
@synackd synackd deleted the case-insensitive-macs branch January 19, 2024 18:36
synackd pushed a commit to synackd/bss that referenced this pull request Feb 29, 2024
Add flag and environment variable for OAUTH2 server
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.

2 participants