-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
metadata/metadata.go
Outdated
} | ||
|
||
// Check against lowercase. | ||
lowerPath := strings.ToLower(req.URL.Path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the metadata server is case sensitive. what is this used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP doesn't specify case sensitivity in its spec, so in case the metadata server decides to stop being case sensitive, we should be aggressive about filtering.
metadata/metadata.go
Outdated
// "service-accounts/<valid-service-account-or-alias>/identity". | ||
// In particular, note that '.' may only come after '@' in the valid | ||
// service account or alias. | ||
concealedPattern = regexp.MustCompile("^\\/(0\\.1\\/meta\\-data|computemetadata\\/v1beta1\\/instance|computemetadata\\/v1\\/instance)/(attributes/kube-env|service-accounts/[0-9a-z_\\-]+(@[0-9a-z_\\-\\.]*)?/identity)$") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are pretty painful. would it simplify things to clean the path first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ptal.
metadata/metadata.go
Outdated
// "service-accounts/<valid-service-account-or-alias>/identity". | ||
// In particular, note that '.' may only come after '@' in the valid | ||
// service account or alias. | ||
concealedPattern = regexp.MustCompile("^\\/(0\\.1\\/meta\\-data|computemetadata\\/v1beta1\\/instance|computemetadata\\/v1\\/instance)/(attributes/kube-env|service-accounts/[0-9a-z_\\-]+(@[0-9a-z_\\-\\.]*)?/identity)$") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
metadata/metadata.go
Outdated
} | ||
|
||
// Check against lowercase. | ||
lowerPath := strings.ToLower(req.URL.Path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP doesn't specify case sensitivity in its spec, so in case the metadata server decides to stop being case sensitive, we should be aggressive about filtering.
/lgtm |
Automatic merge from submit-queue (batch tested with PRs 57906, 57425, 56939, 57317, 57762). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Bump metadata proxy and test versions **What this PR does / why we need it**: Bump metadata proxy version to v0.1.7 (to pick up GoogleCloudPlatform/k8s-metadata-proxy#13). **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes # **Special notes for your reviewer**: **Release note**: ```release-note Bump metadata proxy version to v0.1.7 to pick up security fix. ```
... plus some fixups: