Re: Proposal to move secrets-store-csi-driver to kubernetes-sigs

[Rita Zhang]

Reposting

On Wed, Aug 7, 2019 at 5:02 PM Rita Zhang <rita.z...@gmail.com> wrote:

Hello sig-auth,

 

Over the last few months we have built a tool that integrates off-cluster secret stores (Hashicorp Vault and Azure Key Vault) with Kubernetes deployments through Container Storage Interface (CSI) volumes. This driver allows Kubernetes to mount multiple secrets, keys, and certificates stored in external secret stores as volumes attached to a Pod. The code is available on GitHub (https://github.com/deislabs/secrets-store-csi-driver) and was demoed to SIG Auth on Jan 23. And here is the Kubecon talk we gave at Kubecon Barcelona in May.

 

To continue growing the project, we would like to propose contributing the code to the kubernetes-sigs GitHub organization. We feel that the scope and goals for the secrets store driver are pretty well aligned with SIG Auth. To proceed, we would like to:

• Complete a code review with SIG Auth community
• Establish owners for maintenance/bugs/security/features
• Obtain SIG sponsorship for the repo move to kubernetes-sigs

 

We’d be happy to field any questions/concerns!

Rita

--

Rita Zhang

[Maya Kaczorowski]

No objections from me, would be great to add this to kubernetes-sigs.

Thanks Rita!

Maya Kaczorowski kaczo...@google.com Product Manager, Security & Privacy

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAL7%2BV1wbRbwgZybyRAwTtmcdJ5jdaspcaw-zq5kA1pE1XzO7wA%40mail.gmail.com.

[Craig Peters]

If I understand correctly there is no objection to the secrets-store-csi-driver project or code. Only a question of what org and repo should house the code. I hear that this was a subject of conversation today and I want to reiterate the arguments that I believe Rita already mode for moving from /deislabs to /kubernetes-sigs.

1. Governance: any CSI user could use and contribute, and people are much more likely to contribute to code under CNCF governance
2. Discoverability: including it in the /kubernetes-sigs org will make it much more likely that people who could benefit will find the tool

Leaving the driver in /deislabs would likely mean that most of the contribution would come from Microsoft alone, and I think that the community as a whole wouldn't benefit from that outcome. Please consider this and share your thoughts.

Thanks,

Craig

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CANdooUEFj3ijEps9Kx2i8xHqcjHS9W7AYYftk1gOtyxp9pGAfA%40mail.gmail.com.

[Tasha Drew]

+1 sounds great to me

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAC8XGpEMZ2Ki29VtL4_V4bOtmZYQa4LwgDFwc9%2B1fQU%3D2gM7EQ%40mail.gmail.com.

[Craig Peters]

So what does it take to get to agreement here? Are we good to move the repository?

Thanks,
Craig

On Wednesday, October 2, 2019 at 12:34:24 PM UTC-7, Tasha Drew wrote:

+1 sounds great to me

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-auth+unsub...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAL7%2BV1wbRbwgZybyRAwTtmcdJ5jdaspcaw-zq5kA1pE1XzO7wA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-auth+unsub...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CANdooUEFj3ijEps9Kx2i8xHqcjHS9W7AYYftk1gOtyxp9pGAfA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-auth+unsub...@googlegroups.com.

[David Eads]

I recommend against inclusion.  The stance from sig-storage is that the csi storage drivers should be maintained by the vendors producing them.  That's why the extension mechanism was developed in the first place.  The drivers previously located in k/k were moved and kept under the sigs umbrella to facilitate removing them from k/k, not to act as an example of gathering all csi drivers.

In addition to being against existing guidance from sig-storage, there are several aspects of this particular repo that flag it as not being appropriate to include.

1. The project isn't bringing a community with it.  There are only seven total contributors.  Only two of which have more than 10 commits.
2. The project has only three commits from non-microsoft/non-hashicorp contributors.
3. The project has zero commits from non-microsoft employees in the past two months.

Including a vendor driven project that lacks an opensource community that has aspirations, but little real community backing would give valuable kubernetes branding to a secret store repo that does not have wide backing and adoption.  In light of this, I recommend against having sig-auth sponsor this project.  

I suspect you didn't gain traction in an email thread because even though I think this is fair feedback that I have tried to deliver gently, it's not the answer you were hoping for.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAL7%2BV1wbRbwgZybyRAwTtmcdJ5jdaspcaw-zq5kA1pE1XzO7wA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CANdooUEFj3ijEps9Kx2i8xHqcjHS9W7AYYftk1gOtyxp9pGAfA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAC8XGpEMZ2Ki29VtL4_V4bOtmZYQa4LwgDFwc9%2B1fQU%3D2gM7EQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/69d817da-fc76-4db3-af5c-2d047d34efb1%40googlegroups.com.

[Jay Pipes]

On Mon, 2019-10-14 at 07:48 -0400, David Eads wrote:
> I recommend against inclusion. The stance from sig-storage is that
> the csi storage drivers should be maintained by the vendors producing
> them. That's why the extension mechanism was developed in the first
> place. The drivers previously located in k/k were moved and kept
> under the sigs umbrella to facilitate removing them from k/k, not to
> act as an example of gathering all csi drivers.

David, do you think it would be a good idea to move the AWS-specific
CSI drivers into the github.com/aws organization and out of k8s-sigs?

If so, what machinery is in place to facilitate pkg import paths
(redirecting to a new repo location, etc)?

Best,
-jay

[Rita Zhang]

Thanks for the feedback David, you raised some good points. This proposal to move the repo into Kubernetes-sigs was raised by interested contributors who want this project to be in a neutral home before they can contribute. In the discussions we have had with sig-auth since January, there has been positive feedback from the sig-auth chairs that this project would be a good fit for Kubernetes-sigs. I suggest the best path forward is to have the sig-auth chairs make a decision on whether this should be included.

 

Regarding the issue you raised about csi storage drivers, the project has already split out the providers out of tree. Only the core interfaces is now part of secrets store csi driver project. Provider plugins now all live in their own repos and are managed by individual vendors.

 

Again our goal is to have this project be part of a neutral home for contributors and broader adoption.

Rita

[Mike Danese]

Sorry, I meant to reply to this after last sig meeting and totally forgot.

On Mon, Oct 14, 2019 at 4:49 AM David Eads <de...@redhat.com> wrote:

The stance from sig-storage is that the csi storage drivers should be maintained by the vendors producing them.

https://github.com/kubernetes-sigs/aws-ebs-csi-driver

https://github.com/kubernetes-sigs/alibaba-cloud-csi-driver

https://github.com/kubernetes-sigs/vsphere-csi-driver

https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver

And more...

I agree that broader diversity of contributors would be nice but that's true for any project and I can't discount it on these grounds alone. The project is multi-vendor, aims to simplify the common problem of onboarding k8s integrations with external secret stores, and has contributors willing to contribute. Generally, development in this area is also key to driving storage functionality that benefits other auth related storage integrations, e.g.:

https://github.com/kubernetes/kubernetes/issues/64984

So I would argue that the mere existence of this project benefits the sig-auth and Kubernetes community. Given the alignment of objectives between this project and sig-auth, the community will benefit further by its success.

What the project gets from inclusion as a sig-auth subproject is visibility, access to Kubernetes test infrastructure, and a defined governance model, security process, etc... which all sound like good things. Unfortunately, there's not a lot of guidance in this area, but a secrets driver does not feel at all out of place as a sig subproject and of the various sigs, sig-auth seems like a natural fit.

We have kubernetes-sigs and kubernetes-incubator for exactly this kind of project, and we have kubernetes-retired if it doesn't pan out. Assuming this isn't a zero some game, I'm not seeing a lot of downside to helping this project grow.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAFS1MjK3uEU0jZdef4FuvuJfFOSwuGUW4kPBOxHL3W%3DG_1tK1A%40mail.gmail.com.

[Jay Pipes]

On Tue, 2019-10-15 at 17:14 -0700, Rita Zhang wrote:
> Thanks for the feedback David, you raised some good points. This
> proposal to move the repo into Kubernetes-sigs was raised by
> interested contributors who want this project to be in a neutral home
> before they can contribute. In the discussions we have had with sig-
> auth since January, there has been positive feedback from the sig-
> auth chairs that this project would be a good fit for Kubernetes-
> sigs. I suggest the best path forward is to have the sig-auth chairs
> make a decision on whether this should be included.
>
> Regarding the issue you raised about csi storage drivers, the project
> has already split out the providers out of tree. Only the core
> interfaces is now part of secrets store csi driver project. Provider
> plugins now all live in their own repos and are managed by individual
> vendors.
>
> Again our goal is to have this project be part of a neutral home for
> contributors and broader adoption.

I think the secrets store CSI driver is well-designed and appropriately
decouples the vendor bits from the common bits. The vendor secret store
"providers" live in separate repos from the main secrets-store-csi-
driver. The only vendory things in the secrets-store-csi-driver repo at
all are the links to the Azure and Hashicorp Vault providers in the
README and those provider's associated deployment manifests in the
/deploy directory. I think that's a clean separation.

Best,
-jay

> On Monday, October 14, 2019 at 7:06:35 AM UTC-7, Jay Pipes wrote:
> > On Mon, 2019-10-14 at 07:48 -0400, David Eads wrote:
> > > I recommend against inclusion. The stance from sig-storage is
> > that
> > > the csi storage drivers should be maintained by the vendors
> > producing
> > > them. That's why the extension mechanism was developed in the
> > first
> > > place. The drivers previously located in k/k were moved and
> > kept
> > > under the sigs umbrella to facilitate removing them from k/k, not
> > to
> > > act as an example of gathering all csi drivers.
> >
> > David, do you think it would be a good idea to move the AWS-
> > specific
> > CSI drivers into the github.com/aws organization and out of k8s-
> > sigs?
> >
> > If so, what machinery is in place to facilitate pkg import paths
> > (redirecting to a new repo location, etc)?
> >
> > Best,
> > -jay
> >
>

> --
> You received this message because you are subscribed to the Google
> Groups "kubernetes-sig-auth" group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to kubernetes-sig-...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/kubernetes-sig-auth/41f7022d-da23-4765-9efa-d965146a0658%40googlegroups.com
> .

[Jordan Liggitt]

Rita Zhang wrote: 

This proposal to move the repo into Kubernetes-sigs was raised by interested contributors who want this project to be in a neutral home before they can contribute.

I can appreciate the chicken-and-egg situation (while in the current location, contributors from other companies will be hard to attract). As we're trying to figure out if the SIG has capacity to adopt this project, is there a concrete list of those people with intent to contribute to and own this effort? That's what I was asking for originally with the "establish owners for maintenance/bugs/security/features" request.

Rita Zhang wrote: 

Regarding the issue you raised about csi storage drivers, the project has already split out the providers out of tree. Only the core interfaces is now part of secrets store csi driver project. Provider plugins now all live in their own repos and are managed by individual vendors.

That seems like a positive direction. Can that design point be made explicit in this repo's goals/docs (e.g. vendor-specific providers will not be embedded, and this is a component specific providers can build on top of)? There also still seems to be vendor-specific features listed and manifests included which seem like remnants that should be moved to the provider repos.

Mike Danese wrote:

I would argue that the mere existence of this project benefits the sig-auth and Kubernetes community. Given the alignment of objectives between this project and sig-auth, the community will benefit further by its success.

I agree the alignment is good. I do have questions/observations around capacity/staffing and the current APIs / dependencies / technical debt:

• The original request made reference to completing a code review with SIG Auth community. Has that been done? Was there sufficient bandwidth/engagement there?
• Is there automated CI coverage for end-to-end flows of the component? Can reviewers be confident that merging a PR doesn't break basic functionality based on that CI, or does testing require a downstream provider to update to pick up changes?
• Who (specifically) will be responsible for maintenance/bugs/security/features in this repo? There are multiple sig-auth efforts (CSR, bound service account tokens, dynamic audit policy, etc) already in progress (and somewhat languishing due to lack of bandwidth), so we need to ensure this project brings along staffing to cover the work it adds.
  
• The repo is currently defining a k8s.io-suffixed API, which would need to be renamed to x-k8s.io as it is not an official k8s.io API
• The repo is currently depending on k8s.io/kubernetes, which is not recommended or supported. That dependency is only used for mount utilities, which are being extracted to k8s.io/utils in https://github.com/kubernetes/utils/pull/100. That should be completed and the k8s.io/kubernetes dependency dropped.

[Mo Khan]

Jordan's point about bandwidth is my biggest concern here (I believe the various other technical aspects, especially the x-k8s.io bit, can be addressed):

Who (specifically) will be responsible for maintenance/bugs/security/features in this repo? There are multiple sig-auth efforts (CSR, bound service account tokens, dynamic audit policy, etc) already in progress (and somewhat languishing due to lack of bandwidth), so we need to ensure this project brings along staffing to cover the work it adds.

On a meta level, I would like to understand what the criteria for inclusion are.  From David's comment, I gathered:

1. Needs a (user/dev) community of some size
2. Needs to be vendor neutral
3. Broad set of committers from multiple vendors
4. ... ?
   

By that criteria the project must already be independently successful and the inclusion into kubernetes-sigs is just a formality to state that "we agree that this is the right path."  That is a pretty high bar.  If kubernetes-sigs is meant to represent "the correct way to do something outside of core," I think that bar is correct (and I would not be surprised if people outside of the community see projects in kubernetes-sigs to be "blessed" in some way).  If kubernetes-sigs is more relaxed and has space for the incubation of a project (that may not be successful in the end), then that bar is probably too high.  If at some point in the future we realize that the project did not work out, are we able to archive the repo (or something similar) as a way of undoing the inclusion?

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-auth" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAMBP-p%2BCwVrPCeHoJ%2Be%2B7t9Cv3tgNVwYcgEhSNCX6DPC1TFjig%40mail.gmail.com.

[Tasha Drew]

My few cents on kubernetes-sig repo: for multi-tenancy working group projects, we have several "incubation" projects (some of which are different implementation approaches to the same problem) in kubernetes-sigs/multi-tenancy. We are not using it as a place to be the king maker, we're using it as a shared code repository to work together on different approaches to different problems. I don't think anyone looks at that long list of repos and thinks "this is an enterprise production ready tool set Kubernetes has blessed." If they do, maybe we need to update a readme. The description of the group itself simply states "Org for Kubernetes SIG-related work" 

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAKT2ragXX%3DkcGfOg4icb4-97yvfbx%3DmYE6Da9HWQ%3DdQx7oaWVg%40mail.gmail.com.

[Rita Zhang]

To reply to Jordan's concerns:

> As we're trying to figure out if the SIG has capacity to adopt this project, is there a concrete list of those people with intent to contribute to and own this effort? That's what I was asking for originally with the "establish owners for maintenance/bugs/security/features" request.

I've listed the users in the OWNERS section in this issue https://github.com/kubernetes/org/issues/1245 please lmk if that is not sufficient.

> The original request made reference to completing a code review with SIG Auth community. Has that been done? Was there sufficient bandwidth/engagement there?

It has not. Can you please provide some guidance on how to do this? Who from sig-auth should we work with?

> Is there automated CI coverage for end-to-end flows of the component? Can reviewers be confident that merging a PR doesn't break basic functionality based on that CI, or does testing require a downstream provider to update to pick up changes?

Yes currently CI kicks off e2e tests for every PR to test a full scenario with the Vault provider and every commit in master kicks off e2e tests with the Azure provider. 

> Who (specifically) will be responsible for maintenance/bugs/security/features in this repo?

The users in OWNERS.

All other requests regarding updates in docs, api changes, and dependencies will be addressed soon.

Rita

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-auth/CAAEv1BzyjKDwRhdjJaq8q2eYE6-de4QqYEPnXNkjwKTRkn-%2Bzw%40mail.gmail.com.

--

Rita Zhang

[Jordan Liggitt]

On Wed, Oct 16, 2019 at 7:36 PM Rita Zhang <rita.z...@gmail.com> wrote:

To reply to Jordan's concerns:

As we're trying to figure out if the SIG has capacity to adopt this project, is there a concrete list of those people with intent to contribute to and own this effort? That's what I was asking for originally with the "establish owners for maintenance/bugs/security/features" request.

I've listed the users in the OWNERS section in this issue https://github.com/kubernetes/org/issues/1245 please lmk if that is not sufficient.

Thanks. It looks like the people listed there are all Microsoft/Hashicorp folks, which is not surprising given the current contributor set, but it would be helpful to capture the "interested contributors who want this project to be in a neutral home before they can contribute" somewhere as well.

 

The original request made reference to completing a code review with SIG Auth community. Has that been done? Was there sufficient bandwidth/engagement there?

It has not. Can you please provide some guidance on how to do this? Who from sig-auth should we work with?

I know Mike Danese has interest in this, so I'd suggest setting up a time to walk through the structure and data flow with him. It would also make sense to have someone from sig-storage take a look, since this is building on the CSI mechanism they provide (I copied in some of the CSI subproject owners so they can suggest a good storage contact to take a look). The primary things I would want to see come out of the review:

• The architecture is one we would recommend providers build on
• The approach doesn't expose data inappropriately or require problematic permissions be granted to node-level daemons
• It is using CSI in a way that is supported by sig-storage and doesn't have fundamental problems
• The APIs exposed are acceptable (like switching away from the k8s.io-suffixed API)

[Michelle Au]

I would be happy to review from CSI perspective.

[Rita Zhang]

Thanks Jordan and Michelle. I

 have updated https://github.com/kubernetes/org/issues/1245 with individuals from different companies who have expressed interests in this project being in a neutral home per this discussion. 

I will reach out to Michelle and Mike Danese in the coming weeks to schedule a code review. 

--

Rita Zhang

[Rita Zhang]

Hi everyone, 

We have fixed all the issues identified from the code reviews performed by Mike, Michelle, and Jordan. Thank you for the reviews! Really appreciate the time and the feedback. Here are the issues and PRs closed against each: https://github.com/deislabs/secrets-store-csi-driver/issues?utf8=%E2%9C%93&q=is%3Aissue+%5Bcode+review%5D+

Please let us know if you have other concerns and issues. Also what is our next step to move this repo into kubernetes-sigs. 

Cheers,

Rita

--

Rita Zhang

[Jordan Liggitt]

Thanks for the update. I'll defer to Mike and Michelle if they had a chance to review the resolution of their comments.

>> Regarding the issue you raised about csi storage drivers, the project has already split out the providers out of tree. Only the core interfaces is now part of secrets store csi driver project. Provider plugins now all live in their own repos and are managed by individual vendors.

>

> That seems like a positive direction. Can that design point be made explicit in this repo's goals/docs (e.g. vendor-specific providers will not be embedded, and this is a component specific providers can build on top of)? There also still seems to be vendor-specific features listed and manifests included which seem like remnants that should be moved to the provider repos.

I still had that question about how specific providers will be supported/tested. Currently, the repo indicates two supported providers, and embeds manifests and test jobs for the two providers. Are those manifests considered canonical, or only for test purposes? Would all providers be allowed/encouraged to add themselves to the list of supported providers and contribute merge-blocking CI jobs as well?

[Michelle Au]

I reviewed the csi driver implementation again, and it lgtm from sig-storage.

[Mike Danese]

I reviewed the csi driver implementation for sig-auth and see no issues that should block repo migration. There are a few changes that I believe are blocked on the migration (e.g. complete the migration of CRD name to x-k8s.io?).

[Rita Zhang]

@Mike Danese 

> There are a few changes that I believe are blocked on the migration (e.g. complete the migration of CRD name to x-k8s.io?)

The migration is done. As I mentioned in this issue if you see traces of `k8s.com`, these are the csi driver name secrets-store.csi.k8s.com(not the CRD name), which can only be updated to secrets-store.csi.k8s.io #129 after the repo move, per sig-storage review comment here

@Jordan Liggitt 

> I still had that question about how specific providers will be supported/tested. Currently, the repo indicates two supported providers, and embeds manifests and test jobs for the two providers. Are those manifests considered canonical, or only for test purposes? Would all providers be allowed/encouraged to add themselves to the list of supported providers and contribute merge-blocking CI jobs as well? 

1. To add a new provider to the supported providers list, the provider needs to add themselves to the e2e test suite here that runs on every driver PR to ensure changes in the driver consistently work with the latest stable version of each provider. It is up to the provider maintainer to push the latest stable version of the provider upstream into the driver repo, e.g. here.

2. Each provider is encouraged to use the latest stable driver to its e2e test suite e.g. here that runs on every PR to ensure changes in the provider consistently work with the latest stable version of the driver. 

Hope this helps. Please let me know if there are other questions or concerns.

Rita

--

Rita Zhang

[Jordan Liggitt]

On Thu, Jan 2, 2020 at 9:01 PM Rita Zhang <rita.z...@gmail.com> wrote:

@Jordan Liggitt 

> I still had that question about how specific providers will be supported/tested. Currently, the repo indicates two supported providers, and embeds manifests and test jobs for the two providers. Are those manifests considered canonical, or only for test purposes? Would all providers be allowed/encouraged to add themselves to the list of supported providers and contribute merge-blocking CI jobs as well? 

1. To add a new provider to the supported providers list, the provider needs to add themselves to the e2e test suite here that runs on every driver PR to ensure changes in the driver consistently work with the latest stable version of each provider. It is up to the provider maintainer to push the latest stable version of the provider upstream into the driver repo, e.g. here.

2. Each provider is encouraged to use the latest stable driver to its e2e test suite e.g. here that runs on every PR to ensure changes in the provider consistently work with the latest stable version of the driver. 

Thanks for the info. Having a central manifest in this project that references out to external providers inverts the structure I expected. That leads to questions like:

• What would the criteria be for including new providers in that list? Since that amounts to endorsement/distribution, would there be code audits or quality requirements?
• If a security update is made by a provider, how would that be coordinated with the version-selecting manifest in https://github.com/deislabs/secrets-store-csi-driver/blob/master/charts/secrets-store-csi-driver/values.yaml?
• If changes are made in the future that require provider updates, how would those be coordinated? Would we rev the project to v2, reset the provider list to empty, and let providers add back in v2-compatible versions of themselves?

[Rita Zhang]

Thanks for these questions and apologies for not having these already documented. Let me address each question below and will open a PR to close out the open issue to document how to add a provider. Let us know if you have more questions.

1. The criteria to include a new provider should be as follows:

- code audit of the provider implementation to ensure it adheres to the required provider-driver interface, which includes:

    a. implementation of provider command args https://github.com/deislabs/secrets-store-csi-driver/blob/master/pkg/secrets-store/nodeserver.go#L223-L236

    b. implementation of provider version so driver can check for minimum supported provider version https://github.com/deislabs/secrets-store-csi-driver/blob/master/pkg/secrets-store/nodeserver.go#L215

    c. provider binary naming convention and semver convention

    d. provider binary deployment volume path

    e. implementation of minDriverVersion for bidirectional compatibility checks

    f. provider logs are written to stdout and stderr so they can be part of the driver logs

- the provider needs to add themselves to the e2e test suite here to demonstrate it functions as expected with minimum required driver version

2. If any update is made by a provider (not limited to security updates), the provider is expected to push the latest version of the provider upstream into the driver repo https://github.com/deislabs/secrets-store-csi-driver/blob/master/charts/secrets-store-csi-driver/values.yaml#L24 and if needed, update the --min-provider-version flag here

3. If changes are made in the driver that require provider updates, then the driver maintainers will coordinate with the provider maintainers. With the min-provider-version flag on the driver side, we can perform compatibility checks against the supported list of providers and versions for the latest version of the driver here: https://github.com/deislabs/secrets-store-csi-driver/blob/master/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml#L53-L55

Hope this helps.

--

Rita Zhang

[Jordan Liggitt]

Making this driver the central repository for the deployment manifests for all providers is the inverse of what I expected.

It doesn't scale well to require tagging a release of this component any time any provider updates, or to centralize knowledge of provider compatibility (or embed parameters for all providers into the invocation).

I like the idea of providers being able to (optionally) contribute well-behaved (e.g. non-flaky) e2e tests so we can ensure continued interoperation, but I think providers should own their deployment manifests, not this repo. That removes the need for us to review and make judgement calls about which providers to include/endorse.

[Rita Zhang]

Thanks for the feedback Jordan. I do understand the concerns. 

To clarify, each provider has its own deployment manifest and can be deployed independently. https://github.com/hashicorp/secrets-store-csi-driver-provider-vault/blob/master/deployment/provider-vault-installer.yaml

I think what you are suggesting is remove the deployment of the provider from the driver chart to minimize the dependencies. Is this correct?

 As for provider compatibility and embed parameters for all providers into the invocation, those are nice-to-haves to give the user better error message to prompt for an upgrade. We can make that check optional so it does not block execution if the flag does not contain the provider name. https://github.com/deislabs/secrets-store-csi-driver/blob/master/pkg/secrets-store/nodeserver.go#L207-L209

WDYT?

We will also be on today's sig-auth call in case anyone has more questions or concerns.

--

Rita Zhang

[Rita Zhang]

I think all the issues have been resolved. Can we get a LGTM on https://github.com/kubernetes/org/issues/1245?

Thanks!

--

Rita Zhang

[Craig Peters]

Popping this back up in your inboxes. I think all the i's have been dotted, and the t's have been crossed. As we come to the close of January, it'd be really awesome to finish this up.

Thanks,

Craig

[Jordan Liggitt]

Thanks, I just checked the repo changes and it looks a lot closer to what I expected.

As discussed today in the sig-auth call, it would be good to document criteria for inclusion in the providers list. For example, I would suggest that providers that wish to be included must have a passing integration test (like the azure and vault providers currently do). It would also be good to document the timeframe/procedure for removal from that list if the test starts failing due to the provider, and isn't resolved in a reasonable amount of time.

Other than that, looks good to me. I'll reply on the kubernetes/org issue.

Jordan