cdk8s-team / cdk8s-plus

A software development framework that provides high level abstractions for authoring Kubernetes applications.
https://cdk8s.io/docs/latest/plus/
Apache License 2.0
137 stars 35 forks source link

Lack of clarity on a number of things with the library #1184

Closed ianmurphy1 closed 1 year ago

ianmurphy1 commented 2 years ago

I'm currently using cdk8s along with cdk and so far I'm enjoying it as it feels like an extension of the cdk project that I've been using for a while now.

There is however a serious lack of documentation around the usage of some the constructs and there is little to no examples on their usage as well.

For example, this is a file that i'm using to create some basic k8s resources:- a service account, cluster role and a binding to go with it. It took finally going into the source code in this repo to come up with getting the clusterrole defined and generating the correct yaml.

`import as constructs from 'constructs'; import as cdk8s from 'cdk8s'; import * as kplus from 'cdk8s-plus-23';

export class ExternalDNSChart extends cdk8s.Chart { constructor(scope: constructs.Construct, id: string) { super(scope, id);

const ns = new kplus.Namespace(this, 'Namespace', {
  metadata: {
    name: 'external-dns'
  }
});

const sa = new kplus.ServiceAccount(this, 'ServiceAccount', {
  metadata: {
    name: 'external-dns',
    namespace: 'external-dns'
  }
});

const role = new kplus.ClusterRole(this, 'Role', {
  metadata: {
    name: 'external-dns',
    namespace: 'external-dns'
  },
  rules: [{
    verbs: ['get', 'list', 'watch'],
    endpoints: [{
      asApiResource: () => {
        return {
          apiGroup: '',
          resourceType: 'services'}
      },
      asNonApiResource: () => {return undefined}
    }]
  }]
});

const rb = new kplus.ClusterRoleBinding(this, 'ClusterRoleBinding', {
  role,
  metadata: {
    name: 'eks-admin-binding'
  }
});

rb.addSubjects(sa)

} }`

Note the role.rules[0].endpoints[0] object where I've had to define two functions within the object to return either the rule itself or undefined.

Is this really how role/clusterrole rules should be defined? There are no examples to go from that shows me a better less verbose way to create a clusterrole with rules, if there is can some examples be shown with their usage. I do see there are a number of methods on the Role class that i could also use but these fall into the same issue of have to define an object with two methods:- asApiResource and asNonApiResource present.

Maybe this is an issue that is restricted by kubernetes itself with the way rules are defined but it seems like it's something that could be improved going forward but as I've said before I use the CDK a lot to deploy infrastructure and the documentation on that project far exceeds this one, are there any plans to update the docs within this library to match what the CDK project provides users?

Thanks

iliapolo commented 2 years ago

@ianmurphy1 Thanks for reaching out, and apologies for the delayed response. I acknowledge we need to do better with docs. We did add a few more doc pages recently, and are constantly looking for areas to improve.

I created this issue to help us track it, and you are welcome to comment their as to what type of documentation you'd like to see specifically around RBAC. If you have other doc concerns please feel free to create more issues as well.

In the meantime, if I understand your use-case correctly, I believe you can accomplish it by doing:

const role = new kplus.ClusterRole(chart, 'Role', {
  metadata: {
    name: 'external-dns',
    namespace: 'external-dns',
  },
});

// add the rules to the role, read is shorthand for ['get', 'list', 'watch']
role.allowRead(kplus.ApiResource.SERVICES);

// creates the role binding and adds the service account as a subject
const binding = role.bind(sa);

You definitely do not need to to implement the asApiResource and asNonApiResource methods.

vinayak-kukreja commented 1 year ago

Hey @ianmurphy1, please let us know if this helps your query.

We would be prioritizing documentation, please let us know if you have any other inputs.

ianmurphy1 commented 1 year ago

Hey,

Sorry for not replying @iliapolo it slipped my completely, hence only replying now. I did eventually find the kplus.ApiResource enums as you specified in your reply to get it working in a far less verbose way to what I had in the initial issue, thanks.

It has been a while now that I've gotten to use cdk8s as the project I was using this for in work was parked a while back. For a good example of docs that you should be created then I would look at the CDK project as a guide.

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.SecurityGroup.html

So the example here has a quick example and then the construct props, properties and then the methods. It would be ideal if cdk8s followed the same format with their contructs too. That would result in a far better developer experience when it comes to learning cdk8s.

Thanks

vinayak-kukreja commented 1 year ago

Hey @ianmurphy1, thank you for your input. This is really helpful for us.

For now, l will be closing this issue(in some days, awaiting your confirmation) since it looks like it is resolved for you. I will open a separate issue tracking documentation updates and mention this issue as an example.

github-actions[bot] commented 1 year ago

This issue has not received a response in a while and will be closed soon. If you want to keep it open, please leave a comment below @mentioning a maintainer.