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

⭐️ os cloud resource for azure #5305

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Mar 10, 2025

Part 3 of #5284

This PR enables the new os resource cloud in Azure VMs.

How to test this change

  • Build the os provider (how? docs here)
  • Build the cnquery binary
  • Upload it and open a shell on a cloud instance (ec2, google vm, azure vm) (cnquery shell)
  • Inspect the new cloud resource, try cloud.provider and cloud.instance {*}

Copy link
Contributor

github-actions bot commented Mar 10, 2025

Test Results

3 561 tests  +17   3 557 ✅ +17   1m 59s ⏱️ +14s
  394 suites ± 0       4 💤 ± 0 
   30 files   ± 0       0 ❌ ± 0 

Results for commit 8b43a91. ± Comparison against base commit 07024f9.

♻️ This comment has been updated with latest results.

@afiune afiune force-pushed the afiune/part3/azure/cloud-instance-metadata branch from 3d554d0 to 06318e7 Compare March 10, 2025 21:44
@afiune afiune mentioned this pull request Mar 10, 2025
@afiune afiune force-pushed the afiune/part3/azure/cloud-instance-metadata branch from 06318e7 to 3d39678 Compare March 11, 2025 05:13
Part 3 of #5284

This PR enables the new os resource `cloud` in Azure VMs.

Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune force-pushed the afiune/part3/azure/cloud-instance-metadata branch from 3d39678 to bac882e Compare March 11, 2025 05:26
@@ -787,6 +787,9 @@ func (print *Printer) Data(typ types.Type, data interface{}, codeID string, bund
return print.Secondary(data.(string))

case types.IP:
if data == nil {
return print.Secondary("null")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fixed separately here #5307

@afiune afiune requested review from arlimus and jaym and removed request for arlimus March 11, 2025 05:29
metadataIdentityScriptWindows = `Invoke-RestMethod -TimeoutSec 5 -Headers @{"Metadata"="true"} -Method GET -URI http://169.254.169.254/metadata/instance?api-version=2021-02-01 -UseBasicParsing | ConvertTo-Json`

loadbalancerMetadataScriptUnix = "curl --retry 5 --retry-delay 1 --connect-timeout 1 --retry-max-time 5 --max-time 10 --noproxy '*' -H Metadata:true http://169.254.169.254/metadata/loadbalancer?api-version=2021-02-01"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it ✅

@afiune afiune requested a review from preslavgerchev March 11, 2025 15:33
We are not using version 2023-11-15 since it is still being rolled out and it may not
be available in some regions.

Docs:

https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=windows#supported-api-versions

Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune force-pushed the afiune/part3/azure/cloud-instance-metadata branch from 0e49c08 to 8b43a91 Compare March 11, 2025 15:49
}

// AddOrUpdatePrivateIP adds or updates one or many Ipv4Addresses
func (m *InstanceMetadata) AddOrUpdatePrivateIP(ips ...Ipv4Address) {
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the use case in which we get the same ip twice so it needs to be updated? i'd expect the metadata to give us the IP addresses just once

Copy link
Contributor Author

@afiune afiune Mar 11, 2025

Choose a reason for hiding this comment

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

After doing extensive research, I discovered that the public IP address may come from two different endpoints within the IMDS, either the instance/ metadata or the loadbalander/ one.

Docs:
https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=windows#endpoint-categories

So we don't know where will it come from, but we check both places looking. The question is, what do we do if it is present in both places? I say we merge the information in the best possible way, which is this implementation, but I am open to suggestions.

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