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

[azure] [feat] Add new relationships between network resources #1838

Merged
merged 17 commits into from
Dec 7, 2023

Conversation

1101-1
Copy link
Collaborator

@1101-1 1101-1 commented Nov 24, 2023

Description

  1. Added connect_in_graph method to the network resources.

  2. Also added a tests to the new collected resources.

  3. Fixed error with nested sub-resources.

To-Dos

  • Add test coverage for new or updated functionality
  • Add a tests to check successful connections between network resources
  • Lint and test with tox
  • Add connections between network resources

Code of Conduct

By submitting this pull request, I agree to follow the code of conduct.

Copy link
Member

@aquamatthias aquamatthias left a comment

Choose a reason for hiding this comment

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

Thanks.

Please review the list of edges with respect to successor/predecessor (reverse=true).

I would also question most/all calls that are done in post_process - do we really need them?

Please find my comments.

plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
@aquamatthias
Copy link
Member

And I forgot: AzureSubnet: shouldn't this be a resource?

@1101-1
Copy link
Collaborator Author

1101-1 commented Nov 27, 2023

@aquamatthias

And I forgot: AzureSubnet: shouldn't this be a resource?

We can't take AzureSubnet only by a subscriptionId. Additional parameters required for: resourceGroupNameand virtualNetworkName.

https://learn.microsoft.com/en-us/rest/api/virtualnetwork/subnets/list?view=rest-virtualnetwork-2023-05-01&tabs=HTTP#uri-parameters

URI Parameters

Name In Required Type Description
resourceGroupName path True string The name of the resource group.
subscriptionId path True string The subscription credentials which uniquely identify the Microsoft Azure subscription. The subscription ID forms part of the URI for every service call.
virtualNetworkName path True string The name of the virtual network.
api-version query True string Client API version.

@1101-1 1101-1 force-pushed the network_resources branch 3 times, most recently from a63f52b to a846c9f Compare November 29, 2023 14:04
@1101-1 1101-1 force-pushed the network_resources branch 2 times, most recently from b89a151 to d5f8559 Compare December 1, 2023 14:50
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d842a4) 89.79% compared to head (af539ee) 89.82%.
Report is 19 commits behind head on main.

❗ Current head af539ee differs from pull request most recent head a96ef6f. Consider uploading reports for the commit a96ef6f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1838      +/-   ##
==========================================
+ Coverage   89.79%   89.82%   +0.03%     
==========================================
  Files          91       93       +2     
  Lines       15811    16036     +225     
==========================================
+ Hits        14197    14404     +207     
- Misses       1614     1632      +18     
Flag Coverage Δ
resotocore 89.82% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@1101-1 1101-1 force-pushed the network_resources branch from d5f8559 to 1b88c5e Compare December 1, 2023 15:07
Copy link
Member

@aquamatthias aquamatthias left a comment

Choose a reason for hiding this comment

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

Thanks! A couple of last comments before we can merge it.

plugins/azure/resoto_plugin_azure/azure_client.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/azure_client.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/base.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/base.py Outdated Show resolved Hide resolved
plugins/azure/resoto_plugin_azure/resource/network.py Outdated Show resolved Hide resolved
@1101-1 1101-1 force-pushed the network_resources branch from 6145454 to 15dc3eb Compare December 4, 2023 13:34
@1101-1 1101-1 force-pushed the network_resources branch 3 times, most recently from d078afb to 6975927 Compare December 5, 2023 12:36
@1101-1 1101-1 force-pushed the network_resources branch from 5045449 to 2e40c46 Compare December 5, 2023 13:02
@1101-1 1101-1 force-pushed the network_resources branch from 3761956 to af539ee Compare December 6, 2023 15:55
@aquamatthias aquamatthias merged commit 8111bd9 into someengineering:main Dec 7, 2023
26 checks passed
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