kubernetes-sigs / network-policy-api

This repo addresses further work involving Kubernetes network security beyond the initial NetworkPolicy resource
Apache License 2.0
51 stars 29 forks source link

Add more docs, refactor website, add release tooling #91

Closed astoycos closed 1 year ago

astoycos commented 1 year ago

Partially fixes #59 Fixes #89

netlify[bot] commented 1 year ago

Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name Link
Latest commit bf6f6c21bfd5c1381646dda29bd624ceb004ca28
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/6467c27070c5be0008ddadf3
Deploy Preview https://deploy-preview-91--kubernetes-sigs-network-policy-api.netlify.app/reference/spec
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astoycos

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/network-policy-api/blob/master/OWNERS)~~ [astoycos] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
astoycos commented 1 year ago

/assign @tssurya @npinaeva @Dyanngg

k8s-ci-robot commented 1 year ago

@astoycos: GitHub didn't allow me to assign the following users: npinaeva.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/kubernetes-sigs/network-policy-api/pull/91#issuecomment-1536755681): >/assign @tssurya @npinaeva @Dyanngg Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
astoycos commented 1 year ago

Please take a look at these changes + the new version of the website in the netlify preview and let me know if it needs to change at all :)

Getting this in should allow us to cut a release of v1alpha1 early next week.

astoycos commented 1 year ago

/hold

so that it doesn't automatically merge with an lgtm

astoycos commented 1 year ago

TODO: Refer to all our APIs as NPV2

astoycos commented 1 year ago

Fixed Up following the conversation in our sig-network-policy-api meeting today

tssurya commented 1 year ago

once the cut PR is merged I can do a test in my implementation PR to see if things are working correctly.

shaneutt commented 1 year ago

/cc

astoycos commented 1 year ago

which makes me curious how the yamls were generated... is the pkg/generator/main.go overwriting what we do from codegen? because I see this change in the yaml but we never changed the types.go file so... was the yaml file changes manually done?

So now we don't use the cotroller-gen CLI to do this the generator pkg does it by calling into the controller-gen go package.

        log.Printf("generating CRD for %v\n", groupKind)

        parser.NeedCRDFor(groupKind, nil)
        crdRaw := parser.CustomResourceDefinitions[groupKind]

        // Inline version of "addAttribution(&crdRaw)" ...
        if crdRaw.ObjectMeta.Annotations == nil {
            crdRaw.ObjectMeta.Annotations = map[string]string{}
        }
        crdRaw.ObjectMeta.Annotations[bundleVersionAnnotation] = bundleVersion
        crdRaw.ObjectMeta.Annotations[apiext.KubeAPIApprovedAnnotation] = approvalLink

        // Prevent the top level metadata for the CRD to be generated regardless of the intention in the arguments
        crd.FixTopLevelMetadata(crdRaw)

        conv, err := crd.AsVersion(crdRaw, apiext.SchemeGroupVersion)
        if err != nil {
            log.Fatalf("failed to convert CRD: %s", err)
        }

        out, err := yaml.Marshal(conv)
        if err != nil {
            log.Fatalf("failed to marshal CRD: %s", err)
        }

        fileName := fmt.Sprintf("config/crd/%s_%s.yaml", crdRaw.Spec.Group, crdRaw.Spec.Names.Plural)
        err = os.WriteFile(fileName, out, 0o600)
        if err != nil {
            log.Fatalf("failed to write CRD: %s", err)
        }
    }

Generates the CRDs with the "sigs.k8s.io/controller-tools/pkg/crd" package, adds the specified annotations and writes the CRD to disk

tssurya commented 1 year ago

Generates the CRDs with the "sigs.k8s.io/controller-tools/pkg/crd" package, adds the specified annotations and writes the CRD to disk

+1 way neater. Almost lgtm: I think we still need to add a warning comment for same/notSame labels before the cut:

we probably want to include Dan's suggestion in the description part of same and notsame lables saying "this is subject to change in future, not stable" ?
astoycos commented 1 year ago

+1 @tssurya If it's alright with you I think we can go ahead and merge this, then do a final look at the tooling + any other small API comments before we complete #93

tssurya commented 1 year ago

ack fine with me, let's merge this!

tssurya commented 1 year ago

/lgtm

tssurya commented 1 year ago

/lgtm