-
Notifications
You must be signed in to change notification settings - Fork 5
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 Security Group support #65
Changes from 9 commits
ae224fe
d9d294a
75625c5
26b7f16
b73d9e8
530e669
e0b77b2
8f8bfac
d5c554d
a5679fb
7734fc5
a4b63d4
da5512a
54e2ccd
1afaf73
745388b
2a7c130
02d92ea
06d45e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
--- | ||
apiVersion: apiextensions.k8s.io/v1 | ||
kind: CustomResourceDefinition | ||
metadata: | ||
annotations: | ||
controller-gen.kubebuilder.io/version: v0.16.1 | ||
name: openstacksecuritygroups.region.unikorn-cloud.org | ||
spec: | ||
group: region.unikorn-cloud.org | ||
names: | ||
categories: | ||
- unikorn | ||
kind: OpenstackSecurityGroup | ||
listKind: OpenstackSecurityGroupList | ||
plural: openstacksecuritygroups | ||
singular: openstacksecuritygroup | ||
scope: Namespaced | ||
versions: | ||
- additionalPrinterColumns: | ||
- jsonPath: .metadata.creationTimestamp | ||
name: age | ||
type: date | ||
name: v1alpha1 | ||
schema: | ||
openAPIV3Schema: | ||
description: OpenstackSecurityGroup has no controller, its a database record | ||
of state. | ||
properties: | ||
apiVersion: | ||
description: |- | ||
APIVersion defines the versioned schema of this representation of an object. | ||
Servers should convert recognized schemas to the latest internal value, and | ||
may reject unrecognized values. | ||
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | ||
type: string | ||
kind: | ||
description: |- | ||
Kind is a string value representing the REST resource this object represents. | ||
Servers may infer this from the endpoint the client submits requests to. | ||
Cannot be updated. | ||
In CamelCase. | ||
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | ||
type: string | ||
metadata: | ||
type: object | ||
spec: | ||
properties: | ||
securityGroupID: | ||
description: SecurityGroupID is the security group ID. | ||
type: string | ||
type: object | ||
status: | ||
type: object | ||
required: | ||
- spec | ||
type: object | ||
served: true | ||
storage: true | ||
subresources: {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
--- | ||
apiVersion: apiextensions.k8s.io/v1 | ||
kind: CustomResourceDefinition | ||
metadata: | ||
annotations: | ||
controller-gen.kubebuilder.io/version: v0.16.1 | ||
name: securitygroups.region.unikorn-cloud.org | ||
spec: | ||
group: region.unikorn-cloud.org | ||
names: | ||
categories: | ||
- unikorn | ||
kind: SecurityGroup | ||
listKind: SecurityGroupList | ||
plural: securitygroups | ||
singular: securitygroup | ||
scope: Namespaced | ||
versions: | ||
- additionalPrinterColumns: | ||
- jsonPath: .status.conditions[?(@.type=="Available")].reason | ||
name: status | ||
type: string | ||
- jsonPath: .metadata.creationTimestamp | ||
name: age | ||
type: date | ||
name: v1alpha1 | ||
schema: | ||
openAPIV3Schema: | ||
description: SecurityGroup defines a security group beloning to an identity. | ||
properties: | ||
apiVersion: | ||
description: |- | ||
APIVersion defines the versioned schema of this representation of an object. | ||
Servers should convert recognized schemas to the latest internal value, and | ||
may reject unrecognized values. | ||
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | ||
type: string | ||
kind: | ||
description: |- | ||
Kind is a string value representing the REST resource this object represents. | ||
Servers may infer this from the endpoint the client submits requests to. | ||
Cannot be updated. | ||
In CamelCase. | ||
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | ||
type: string | ||
metadata: | ||
type: object | ||
spec: | ||
properties: | ||
ingress: | ||
description: Ingress are the ingress rules. | ||
items: | ||
properties: | ||
port: | ||
description: Port is the port or range of ports. | ||
properties: | ||
number: | ||
description: Number is the port number. | ||
type: integer | ||
range: | ||
description: Range is the port range. | ||
properties: | ||
end: | ||
description: End is the end of the range. | ||
maximum: 65535 | ||
type: integer | ||
start: | ||
description: Start is the start of the range. | ||
minimum: 1 | ||
type: integer | ||
required: | ||
- end | ||
- start | ||
type: object | ||
required: | ||
- number | ||
- range | ||
type: object | ||
protocol: | ||
description: Protocol is the protocol of the rule. | ||
enum: | ||
- tcp | ||
- udp | ||
type: string | ||
required: | ||
- port | ||
- protocol | ||
type: object | ||
type: array | ||
pause: | ||
description: Pause, if true, will inhibit reconciliation. | ||
type: boolean | ||
provider: | ||
description: Provider defines the provider type. | ||
enum: | ||
- openstack | ||
type: string | ||
tags: | ||
description: |- | ||
Tags are an abitrary list of key/value pairs that a client | ||
may populate to store metadata for the resource. | ||
items: | ||
description: Tag is an arbirary key/value. | ||
properties: | ||
name: | ||
description: Name of the tag. | ||
type: string | ||
value: | ||
description: Value of the tag. | ||
type: string | ||
required: | ||
- name | ||
- value | ||
type: object | ||
type: array | ||
required: | ||
- provider | ||
type: object | ||
status: | ||
properties: | ||
conditions: | ||
description: Current service state of a cluster manager. | ||
items: | ||
description: |- | ||
Condition is a generic condition type for use across all resource types. | ||
It's generic so that the underlying controller-manager functionality can | ||
be shared across all resources. | ||
properties: | ||
lastTransitionTime: | ||
description: Last time the condition transitioned from one status | ||
to another. | ||
format: date-time | ||
type: string | ||
message: | ||
description: Human-readable message indicating details about | ||
last transition. | ||
type: string | ||
reason: | ||
description: Unique, one-word, CamelCase reason for the condition's | ||
last transition. | ||
enum: | ||
- Provisioning | ||
- Provisioned | ||
- Cancelled | ||
- Errored | ||
- Deprovisioning | ||
- Deprovisioned | ||
type: string | ||
status: | ||
description: |- | ||
Status is the status of the condition. | ||
Can be True, False, Unknown. | ||
type: string | ||
type: | ||
description: Type is the type of the condition. | ||
enum: | ||
- Available | ||
type: string | ||
required: | ||
- lastTransitionTime | ||
- message | ||
- reason | ||
- status | ||
- type | ||
type: object | ||
type: array | ||
type: object | ||
required: | ||
- spec | ||
type: object | ||
served: true | ||
storage: true | ||
subresources: | ||
status: {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ limitations under the License. | |
package v1alpha1 | ||
|
||
import ( | ||
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/rules" | ||
unikornv1core "github.com/unikorn-cloud/core/pkg/apis/unikorn/v1alpha1" | ||
|
||
"k8s.io/apimachinery/pkg/api/resource" | ||
|
@@ -517,3 +518,103 @@ type FlavorQuota struct { | |
|
||
type QuotaStatus struct { | ||
} | ||
|
||
// SecurityGroupList is a typed list of security groups. | ||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
type SecurityGroupList struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ListMeta `json:"metadata,omitempty"` | ||
Items []SecurityGroup `json:"items"` | ||
} | ||
|
||
// SecurityGroup defines a security group beloning to an identity. | ||
// +genclient | ||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
// +kubebuilder:resource:scope=Namespaced,categories=unikorn | ||
// +kubebuilder:subresource:status | ||
// +kubebuilder:printcolumn:name="status",type="string",JSONPath=".status.conditions[?(@.type==\"Available\")].reason" | ||
// +kubebuilder:printcolumn:name="age",type="date",JSONPath=".metadata.creationTimestamp" | ||
type SecurityGroup struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ObjectMeta `json:"metadata,omitempty"` | ||
Spec SecurityGroupSpec `json:"spec"` | ||
Status SecurityGroupStatus `json:"status,omitempty"` | ||
} | ||
|
||
type SecurityGroupSpec struct { | ||
// Pause, if true, will inhibit reconciliation. | ||
Pause bool `json:"pause,omitempty"` | ||
// Tags are an abitrary list of key/value pairs that a client | ||
// may populate to store metadata for the resource. | ||
Tags TagList `json:"tags,omitempty"` | ||
// Provider defines the provider type. | ||
Provider Provider `json:"provider"` | ||
// Ingress are the ingress rules. | ||
Ingress []SecurityGroupRule `json:"ingress,omitempty"` | ||
} | ||
|
||
// +kubebuilder:validation:Enum=tcp;udp | ||
type SecurityGroupRuleProtocol string | ||
|
||
const ( | ||
TCP SecurityGroupRuleProtocol = "tcp" | ||
UDP SecurityGroupRuleProtocol = "udp" | ||
) | ||
|
||
type SecurityGroupRulePortRange struct { | ||
// Start is the start of the range. | ||
// +kubebuilder:validation:Minimum=1 | ||
Start int `json:"start"` | ||
// End is the end of the range. | ||
// +kubebuilder:validation:Maximum=65535 | ||
End int `json:"end"` | ||
} | ||
|
||
type SecurityGroupRulePort struct { | ||
// Number is the port number. | ||
Number *int `json:"number"` | ||
// Range is the port range. | ||
Range *SecurityGroupRulePortRange `json:"range"` | ||
} | ||
|
||
type SecurityGroupRule struct { | ||
// Protocol is the protocol of the rule. | ||
Protocol SecurityGroupRuleProtocol `json:"protocol"` | ||
// Port is the port or range of ports. | ||
Port SecurityGroupRulePort `json:"port"` | ||
} | ||
|
||
type SecurityGroupStatus struct { | ||
// Current service state of a cluster manager. | ||
Conditions []unikornv1core.Condition `json:"conditions,omitempty"` | ||
} | ||
|
||
// OpenstackSecurityGroupList is a typed list of security groups. | ||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
type OpenstackSecurityGroupList struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ListMeta `json:"metadata,omitempty"` | ||
Items []OpenstackSecurityGroup `json:"items"` | ||
} | ||
|
||
// OpenstackSecurityGroup has no controller, its a database record of state. | ||
// +genclient | ||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
// +kubebuilder:resource:scope=Namespaced,categories=unikorn | ||
// +kubebuilder:printcolumn:name="age",type="date",JSONPath=".metadata.creationTimestamp" | ||
type OpenstackSecurityGroup struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ObjectMeta `json:"metadata,omitempty"` | ||
Spec OpenstackSecurityGroupSpec `json:"spec"` | ||
Status OpenstackSecurityGroupStatus `json:"status,omitempty"` | ||
} | ||
|
||
type OpenstackSecurityGroupSpec struct { | ||
// SecurityGroupID is the security group ID. | ||
SecurityGroupID *string `json:"securityGroupID,omitempty"` | ||
// Rules are the security group rules. | ||
Rules []rules.SecGroupRule `json:"rules,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should be importing/exposing gophercloud types, there's jsut way too much unnecessary crap in there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😮 I updated it but forgot to push. The gophercloud type is missing the JSON tag on a few elements, which is causing the CRD generation to fail. I'll fix that - thanks for the review so far! ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to keep on top of it! It's a pain if you're having to use replace in go.mod too often... here's something I find useful: #!/bin/bash
export GOPRIVATE=github.com/unikorn-cloud
go mod edit -replace $1=$1@$2
go mod tidy |
||
} | ||
|
||
type OpenstackSecurityGroupStatus struct { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the best thing we can do here for general cleanliness and ease of use is to have each rule have a unique identifier and also a description e.g.
The description is just to make the UX a lot better, sure Adam will thank you in future!
The ID however can be used in the mapping from unikorn to openstack e.g.
I think this will improve the reconcile logic a whole heap as you're just concerned with IDs and not the whole tuple of direction, port range, protocol etc etc. Obviously the API will need to be tweaked to be:
A call to create a rule will generate a unique UUID like creation of the security group itself. I am aware of time, so rather than have rules as a separate CRD and controller we leave that bit the way it is.
As I always say, if the data structure is not quite right, the code will suck!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestions! Implementing a unique identifier for each rule did help simplify the reconciliation logic somewhat, but splitting the CRDs made a bigger difference. I ended up splitting the CRDs and moving the security group reconciliation to its own controller, which eliminated the need to iterate over the list of rules entirely.