Whitelisting Onelake API & Workspace PL FQDNs #552
Conversation
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @SmritiAgrawal04
This PR needs some tests to show it working I think
crepererum
left a comment
crepererum
left a comment
There was a problem hiding this comment.
I do somewhat agree with @alamb. I know it's close to impossible to write an integration test for this, but maybe we can at least have a unit test for parse_url?
| let first_label = host.split('.').next().unwrap_or_default(); | ||
| self.account_name = Some(validate(first_label)?); | ||
|
|
||
| let container = parsed.path_segments().unwrap().next().expect( |
There was a problem hiding this comment.
nit: onelake uses workspace terminology
| // Regex to match WS-PL FQDN: "{workspaceid}.z??.dfs.fabric.microsoft.com" | ||
| // workspaceid = 32 hex chars, z?? = z + first two chars of workspaceid | ||
| lazy_static::lazy_static! { | ||
| static ref WS_PL_REGEX: Regex = Regex::new(r"^(?P<workspaceid>[0-9a-f]{32})\.z(?P<xy>[0-9a-f]{2})\.(dfs|blob)\.fabric\.microsoft\.com$").unwrap(); |
There was a problem hiding this comment.
Let's add support for .onelake.fabric.microsoft.com also
| let xy = captures.name("xy").unwrap().as_str(); | ||
|
|
||
| // Validate z?? matches first 2 chars of workspaceid | ||
| if &workspaceid[0..2] != xy { |
| } | ||
|
|
||
| // Otherwise, check Fabric global / Onelake API FQDN | ||
| if host.ends_with(DFS_FABRIC_SUFFIX) || host.ends_with(BLOB_FABRIC_SUFFIX) { |
There was a problem hiding this comment.
if we are checking for global endpoint, should these not be with not like (!host.ends_with(DFS_FABRIC_SUFFIX) && !host.ends_with(BLOB_FABRIC_SUFFIX))
| if host.ends_with(DFS_FABRIC_SUFFIX) || host.ends_with(BLOB_FABRIC_SUFFIX) { | ||
| let labels: Vec<&str> = host.split('.').collect(); | ||
| let account_name = if labels.len() >= 2 && labels[0].contains("api") && labels[1] == "onelake" { | ||
| format!("{}-{}", labels[0], labels[1]) |
There was a problem hiding this comment.
If we are referring workspace id as account_name, then this will not work for non pl scenario. In those case it will give account_name as "westus-api-onelake"
4 participants
Which issue does this PR close?
Closes #.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?