Add ORC API linter to enforce ORC specific rules #677
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
I didn't know that we could create a custom linter; that's cool. There is a lot to learn.
The code looks good to me overall, just left a small observation.
| // not OpenStack resource references. | ||
| var excludedIDPatterns = []string{ | ||
| "SegmentationID", // VLAN segmentation ID, not an OpenStack resource | ||
| "SegmentationIDs", // Plural form of the above |
Contributor
There was a problem hiding this comment.
Having this one here does not hurt, but we don't have a reference for this word throughout the code. Do we really need to exclude this pattern?
Collaborator
Author
There was a problem hiding this comment.
Right, there's no use for it, and probably won't be. I'll drop it.
|
Failed to assess the semver bump. See logs for details. |
Add a custom golangci-lint plugin that flags OpenStack ID references in spec structs, enforcing ORC's API design philosophy that spec fields should only reference ORC Kubernetes objects, not OpenStack resources directly by UUID. The noopenstackidref linter flags fields like 'ProjectID *string' in spec/filter structs and suggests using '*KubernetesNameRef' with a 'Ref' suffix instead (e.g., 'ProjectRef *KubernetesNameRef'). Status structs are exempt, as they are expected to report OpenStack UUIDs. See: https://k-orc.cloud/development/architecture/#api-design-philosophy
Fields like NetworkIDs, SubnetIDs should use NetworkRefs, SubnetRefs instead. This ensures consistency with the singular form (NetworkID -> NetworkRef).
Fields ending in Ref or Refs should use KubernetesNameRef type to reference ORC objects. This catches cases where the naming convention is correct but the type is wrong (e.g., SecurityGroupRefs []OpenStackName should be []KubernetesNameRef). CloudCredentialsRef is excluded as it intentionally uses a different type.
- HostID.ID: Allows raw hypervisor host ID as alternative to ServerRef - PortResourceSpec.HostID: The HostID struct intentionally provides both options - SecurityGroupRefs: Known issue k-orc#438, breaking change planned for next API version
Update API design documentation to align with ssatags linter behavior: listType=set is discouraged for object arrays due to SSA merge issues, so recommend listType=map for structs and listType=set only for primitives.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a custom
noopenstackidreflinter to kube-api-lint that enforces ORC's API design philosophy: spec and filter fields should reference ORC Kubernetes objects using*KubernetesNameRefwith a Ref suffix rather than OpenStack resources directly by UUID.