Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configuration docs for API restrictions #24968

Merged
merged 4 commits into from May 30, 2023

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Apr 19, 2023

Requires: #24967, #25009

This is the documentation update corresponding to #25009.

The following commits are new:

  • api: Add 403 forbidden error codes
  • api: Update autogenerated API code for 403 errors
  • tools: Add cilium-api-flaggen
  • docs: Add configuration docs for API restrictions

This PR is quite big, but primarily due to the auto-generated second commit which is primarily included in order to update this page: https://docs.cilium.io/en/v1.13/api/#api-ref .

@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Apr 19, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Apr 19, 2023
@joestringer joestringer reopened this Apr 20, 2023
@joestringer
Copy link
Member Author

/test

@joestringer joestringer force-pushed the pr/joe/api-restrictions-doc branch from 09f9732 to c1c16bf Compare May 2, 2023 01:23
@joestringer
Copy link
Member Author

joestringer commented May 2, 2023

/test

EDIT: net-next job hit #23175

Job 'Cilium-PR-K8s-1.24-kernel-5.4' failed:

Click to show.

Test Name

K8sDatapathConfig MonitorAggregation Checks that monitor aggregation restricts notifications

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.24-kernel-5.4/src/github.com/cilium/cilium/test/k8s/manifests/l3-policy-demo.yaml: Cannot retrieve "cilium-p7vjf"'s policy revision: cannot get policy revision: ""

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-5.4/156/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.24-kernel-5.4 so I can create one.

Then please upload the Jenkins artifacts to that issue.

@joestringer joestringer marked this pull request as ready for review May 2, 2023 06:49
@joestringer joestringer requested review from a team as code owners May 2, 2023 06:49
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joestringer Good work. 👍🏻 Some things to work in here and in future PRs:

  • Use active voice
  • Address the user as "you"
  • Use present tense

Comment on lines +12 to +20
DeleteEndpointID Deletes the endpoint specified by the ID. Deletion is
imminent and atomic, if the deletion request is valid and
the endpoint exists, deletion will occur even if errors are
encountered in the process. If errors have been encountered,
the code 202 will be returned, otherwise 200 on success. All
resources associated with the endpoint will be freed and the
workload represented by the endpoint will be disconnected.It
will no longer be able to initiate or receive communications
of any sort.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edits for clarity, use present tense

Suggested change
DeleteEndpointID Deletes the endpoint specified by the ID. Deletion is
imminent and atomic, if the deletion request is valid and
the endpoint exists, deletion will occur even if errors are
encountered in the process. If errors have been encountered,
the code 202 will be returned, otherwise 200 on success. All
resources associated with the endpoint will be freed and the
workload represented by the endpoint will be disconnected.It
will no longer be able to initiate or receive communications
of any sort.
DeleteEndpointID Deletes the endpoint specified by the ID. Deletion is
imminent and atomic: if the deletion request is valid and
the endpoint exists, deletion occurs even if errors are
encountered in the process. If errors are encountered,
HTTP code 202 is returned, otherwise 200 returns on success. Upon deletion,
all resources associated with the endpoint are freed and the
workload represented by the endpoint is disconnected. The endpoint
is no longer able to initiate or receive communications
of any sort.

Comment on lines +21 to +23
DeleteFqdnCache Deletes matching DNS lookups from the cache, optionally
restricted by DNS name. The removed IP data will no longer
be used in generated policies.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarity, present tense

Suggested change
DeleteFqdnCache Deletes matching DNS lookups from the cache, optionally
restricted by DNS name. The removed IP data will no longer
be used in generated policies.
DeleteFqdnCache Deletes the matching DNS lookups from the cache, optionally
restricted by DNS name. The removed IP data is no longer
used in generated policies.

Comment on lines +29 to +31
GetBGPPeers Retrieves current operational state of BGP peers created by
Cilium BGP virtual router. This includes session state,
uptime, information per address family, etc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarity, no Latin abbreviations

Suggested change
GetBGPPeers Retrieves current operational state of BGP peers created by
Cilium BGP virtual router. This includes session state,
uptime, information per address family, etc.
GetBGPPeers Retrieves the current operational state of BGP peers created by
the Cilium BGP virtual router. This includes session state,
uptime, information per address family, and others.

GetEndpoint Retrieves a list of endpoints that have metadata matching
the provided parameters, or all endpoints if no parameters
provided.
GetEndpointID Returns endpoint information
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use periods consistently

Suggested change
GetEndpointID Returns endpoint information
GetEndpointID Returns endpoint information.

Comment on lines +55 to +58
GetIP Retrieves a list of IPs with known associated information
such as their identities, host addresses, Kubernetes pod
names, etc. The list can optionally filtered by a CIDR IP
range.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No Latin abbreviations

Suggested change
GetIP Retrieves a list of IPs with known associated information
such as their identities, host addresses, Kubernetes pod
names, etc. The list can optionally filtered by a CIDR IP
range.
GetIP Retrieves a list of IPs with known associated information
such as their identities, host addresses, Kubernetes pod
names, and others. The list can optionally filtered by a CIDR IP
range.

PatchPrefilter -
PostIPAM -
PostIPAMIP -
PutEndpointID Creates a new endpoint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use periods consistently

Suggested change
PutEndpointID Creates a new endpoint
PutEndpointID Creates a new endpoint.

===================== ====================
Flag Name Description
===================== ====================
GetHealthz This path will return the status of cilium operator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use present tense, clarity

Suggested change
GetHealthz This path will return the status of cilium operator
GetHealthz This path returns the status of the cilium operator

Documentation/configuration/api-restrictions.rst Outdated Show resolved Hide resolved
Documentation/configuration/api-restrictions.rst Outdated Show resolved Hide resolved
Documentation/configuration/api-restrictions.rst Outdated Show resolved Hide resolved
@joestringer
Copy link
Member Author

joestringer commented May 2, 2023

Thanks for the feedback @zacharysarah . I've applied all of the suggestions in the new text. For the suggestions in api-restrictions-table.rst, I suggest we follow up separately since all of that text is auto-generated from existing text samples elsewhere in the tree; this PR is just consuming them. Once we've updated the source, we can re-generate these files to update them. I'll push a fresh version shortly.

@joestringer joestringer force-pushed the pr/joe/api-restrictions-doc branch from c1c16bf to 72a607f Compare May 2, 2023 23:24
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joestringer Thanks for your patience and sorry for the long delay. LGTM from a docs perspective.

Add these to all PATCH/POST/PUT/DELETE APIs.

Signed-off-by: Joe Stringer <joe@cilium.io>
This was generated by running "make generate-api" and
"make generate-health-api".

Signed-off-by: Joe Stringer <joe@cilium.io>
This new tool can generate tables with the details about the new flag
options that can be used to administratively enable select APIs.

Signed-off-by: Joe Stringer <joe@cilium.io>
Co-authored-by: Sarah Corleissen <sarah.corleissen@isovalent.com>
Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer
Copy link
Member Author

Half of CI (jenkins part) triggered automatically, so I guess I'll trigger the rest.

@joestringer
Copy link
Member Author

/ci-e2e

@joestringer
Copy link
Member Author

/ci-l4lb

@joestringer
Copy link
Member Author

/ci-multicluster

@joestringer
Copy link
Member Author

/ci-awscni

@joestringer
Copy link
Member Author

/ci-eks

@joestringer
Copy link
Member Author

/ci-verifier

@joestringer
Copy link
Member Author

/ci-external-workloads

@joestringer
Copy link
Member Author

joestringer commented May 26, 2023

/mlh new-flake Cilium-PR-K8s-1.24-kernel-5.4

👍 created #25721

@joestringer
Copy link
Member Author

joestringer commented May 26, 2023

/test-1.24-5.4

Job 'Cilium-PR-K8s-1.24-kernel-5.4' failed:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: Unexpected missed tail call

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-5.4/187/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.24-kernel-5.4 so I can create one.

Then please upload the Jenkins artifacts to that issue.

@joestringer
Copy link
Member Author

k8s-1.24-kernel-5.4 (test-1.24-5.4) failure is #24514

@joestringer
Copy link
Member Author

/test-1.24-5.4

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 26, 2023
@joestringer joestringer merged commit 57f6f04 into cilium:main May 30, 2023
57 checks passed
@joestringer joestringer deleted the pr/joe/api-restrictions-doc branch May 30, 2023 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants