search

kubernetes-retired / spartakus

[EOL] Anonymous Usage Collector
Apache License 2.0
74 stars 30 forks source link

report: add cloud provider name to report #15

Closed squat closed 7 years ago

squat commented 7 years ago

This commit modifies the Node struct and adds a new field CloudProvider. This value is determined by taking the <ProviderName> portion of the ProviderID reported in the node spec. Making this change will require updating the database schema. The cloud provider name is not PII and will provide useful insight into how people are using Kubernetes.

Addresses: #14

philips commented 7 years ago

LGTM

thockin commented 7 years ago

What is the provider name for on-prem installations? How do we know that is not PII ?

On Fri, Dec 2, 2016 at 12:13 PM, Lucas Servén notifications@github.com wrote:

This commit modifies the Node struct and adds a new field CloudProvider. This value is determined by taking the portion of the ProviderID reported in the node spec. Making this change will require updating the database schema. The cloud provider name is not PII and will provide useful insight into how people are using Kubernetes.

Addresses: #14 https://github.com/kubernetes-incubator/spartakus/issues/14

You can view, comment on, or merge this pull request online at:

https://github.com/kubernetes-incubator/spartakus/pull/15 Commit Summary

  • report: add cloud provider name to report

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVKLzHDHdQghuogR9-OHvcK1Dokbmks5rEHvfgaJpZM4LC8ep .

philips commented 7 years ago

@thockin hrm, good point, maybe we whitelist the 7 well-known existing cloud implementations?

thockin commented 7 years ago

or hash it and just depend on some cloudproviders having well-known hashes?

On Fri, Dec 2, 2016 at 1:46 PM, Brandon Philips notifications@github.com wrote:

@thockin https://github.com/thockin hrm, good point, maybe we whitelist the 7 well-known existing cloud implementations?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264571253, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVKldVBbaPuN1GxLvi5QhrcARfmDLks5rEJHOgaJpZM4LC8ep .

thockin commented 7 years ago

We need to be SUPER DUPER EXTRA CAUTIOUS about PII. Please please please.

On Fri, Dec 2, 2016 at 4:53 PM, Tim Hockin thockin@google.com wrote:

or hash it and just depend on some cloudproviders having well-known hashes?

On Fri, Dec 2, 2016 at 1:46 PM, Brandon Philips notifications@github.com wrote:

@thockin https://github.com/thockin hrm, good point, maybe we whitelist the 7 well-known existing cloud implementations?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264571253, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVKldVBbaPuN1GxLvi5QhrcARfmDLks5rEJHOgaJpZM4LC8ep .

philips commented 7 years ago

@thockin totally agree on being cautious. I hadn't even thought that someone might fork and create their own cloud provider.

I feel a static whitelist and saying "unknown" if it isn't int he whitelist is the least risk PII leak situation.

thockin commented 7 years ago

But eventually cloudproviders will be out-of-tree so a whitelist is a dead-end

On Fri, Dec 2, 2016 at 5:06 PM, Brandon Philips notifications@github.com wrote:

@thockin https://github.com/thockin totally agree on being cautious. I hadn't even thought that someone might fork and create their own cloud provider.

I feel a static whitelist and saying "unknown" if it isn't int he whitelist is the least risk PII leak situation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264602648, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVPP7UyOGQOwXUe7coIaMG_NM54cpks5rEMClgaJpZM4LC8ep .

philips commented 7 years ago

@thockin well, with a whitelist we can put in the README exactly what we collect and don't. With a hash it only takes someone leaking the name of their private cloud provider once on some mailing list to unlock all of the old spartakus data.

thockin commented 7 years ago

fair enough. I can live with a whitelist, and an occasional PR to add items to it.

On Fri, Dec 2, 2016 at 5:54 PM, Brandon Philips notifications@github.com wrote:

@thockin https://github.com/thockin well, with a whitelist we can put in the README exactly what we collect and don't. With a hash it only takes someone leaking the name of their private cloud provider once on same mailing list to unlock all of the old spartakus data.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264607086, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVOOGh3Jze8LzhQRsaybd_4ewfXJIks5rEMvdgaJpZM4LC8ep .

squat commented 7 years ago

Sounds good. I'll update the PR to use a whitelist 👍

On Sat, Dec 3, 2016 at 10:22 PM Tim Hockin notifications@github.com wrote:

fair enough. I can live with a whitelist, and an occasional PR to add items to it.

On Fri, Dec 2, 2016 at 5:54 PM, Brandon Philips notifications@github.com wrote:

@thockin https://github.com/thockin well, with a whitelist we can put in the README exactly what we collect and don't. With a hash it only takes someone leaking the name of their private cloud provider once on same mailing list to unlock all of the old spartakus data.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264607086 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AFVgVOOGh3Jze8LzhQRsaybd_4ewfXJIks5rEMvdgaJpZM4LC8ep

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264687049, or mute the thread https://github.com/notifications/unsubscribe-auth/ATiQP26X3wYpEBLt0Endwcyos25rYkEcks5rElwvgaJpZM4LC8ep .

thockin commented 7 years ago

please ping me on it so I can update the DB schema.

On Sat, Dec 3, 2016 at 10:42 PM, Lucas Servén notifications@github.com wrote:

Sounds good. I'll update the PR to use a whitelist 👍

On Sat, Dec 3, 2016 at 10:22 PM Tim Hockin notifications@github.com wrote:

fair enough. I can live with a whitelist, and an occasional PR to add items to it.

On Fri, Dec 2, 2016 at 5:54 PM, Brandon Philips < notifications@github.com> wrote:

@thockin https://github.com/thockin well, with a whitelist we can put in the README exactly what we collect and don't. With a hash it only takes someone leaking the name of their private cloud provider once on same mailing list to unlock all of the old spartakus data.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/kubernetes-incubator/spartakus/pull/15# issuecomment-264607086 , or mute the thread < https://github.com/notifications/unsubscribe- auth/AFVgVOOGh3Jze8LzhQRsaybd_4ewfXJIks5rEMvdgaJpZM4LC8ep

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15# issuecomment-264687049, or mute the thread https://github.com/notifications/unsubscribe-auth/ ATiQP26X3wYpEBLt0Endwcyos25rYkEcks5rElwvgaJpZM4LC8ep .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/pull/15#issuecomment-264687689, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVD5Nfq4PdsmvQVTp-ckuWPEk93Q9ks5rEmDKgaJpZM4LC8ep .