Skip to content

Support new graph and config formats used by shadow#31

Merged
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:new-graph-format
Aug 23, 2021
Merged

Support new graph and config formats used by shadow#31
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:new-graph-format

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler commented Aug 19, 2021

Support the new shadow graph and configuration formats. We now assign hosts directly to graph nodes rather than using hints like "country_code_hint" and "ip_address_hint", so we now need to read and use the network graph.

@robgjansen Let me know if you'd like me to rename any of the options or filenames. Also I think I replicated the host assignment logic to be the same as it is in shadow, but make sure it aligns with how you expect it to run,

@stevenengler stevenengler added the enhancement New feature or request label Aug 19, 2021
@stevenengler stevenengler self-assigned this Aug 19, 2021
@stevenengler stevenengler force-pushed the new-graph-format branch 2 times, most recently from 9a7847e to a4c9fdd Compare August 19, 2021 16:22
Copy link
Copy Markdown
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

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

Overall I think this is great! I'm really happy that you were able to compress and simplify the filtering logic compared to the C version.

You may want to add some comments, and double check the longest prefix match logic when two nodes have the same longest prefix from the ip address hint.

@stevenengler
Copy link
Copy Markdown
Contributor Author

I also fixed the IP prefix match, which was incorrect since Python was interpreting the results of the bitwise operations as a signed integer rather than unsigned.

@stevenengler stevenengler merged commit e5055e5 into shadow:main Aug 23, 2021
@stevenengler stevenengler deleted the new-graph-format branch August 23, 2021 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants