You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/09/23 05:28:38 UTC

[GitHub] [apisix-ingress-controller] neverCase opened a new pull request #689: fix: init ApisixPluginConfig crd (#638)

neverCase opened a new pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   - Related issues
   #638 
   ___
   ### Bugfix
   - Description
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   
   ___
   ### Backport patches
   - Why need to backport?
   
   - Source branch
   
   - Related commits and pull requests
   
   - Target branch
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714493943



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced

Review comment:
       IMHO it should be a namespaced resource.

##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`

Review comment:
       We may add more constraints for this structure. See the JSON schema for it: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L830

##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins,omitempty" yaml:"plugins,omitempty"`
+}
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// ApisixPluginConfigList contains a list of ApisixPluginConfig.
+type ApisixPluginConfigList struct {

Review comment:
       Is this no longer used? Or how do we list ApisixPluginConfig?

##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,64 @@
+#

Review comment:
       Is this generated by `make codegen`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716128586



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`
+	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	// Must be required.

Review comment:
       ok, got it




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927223400


   > The structure of ApisixPluginConfigSpec can be as follows, it only needs to include the following two fields. (you can add some comments)
   > 
   > ```go
   > type ApisixPluginConfigSpec struct {
   > 	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
   > 	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins" yaml:"plugins"`
   > }
   > ```
   
   but the `ApisixRouteHTTPPluginConfig` and `ApisixStatus` don't exist in the new created file `pkg/kube/apisix/apis/config/v1alpha1/types.go`, 
   so should i import them from the `pkg/kube/apisix/apis/config/v2alpha1/types.go`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926531492


   @neverCase  you should re-run `make codegen` and push it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] gxthrj commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716025671



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: apisixpluginconfigs
+    singular: apisixpluginconfigs
+    kind: ApisixPluginConfig
+    shortNames:
+      - apc
+  preserveUnknownFields: false
+  subresources:
+    status: {}
+  validation:
+    openAPIV3Schema:
+      type: object
+      properties:
+        spec:
+          type: object
+          oneOf:
+            - required: ["id","plugins"]
+          properties:
+            id:
+              type: integer
+              minimum: 0
+            labels:
+              type: object
+              items:
+                type: string
+            desc:
+              type: string
+              minLength: 1
+            plugins:
+              type: array

Review comment:
       ```suggestion
                 type: array
                 minItems: 1
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r715504421



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: apisixpluginconfigs
+    singular: apisixpluginconfigs
+    kind: ApisixPluginConfig
+    shortNames:
+      - apc
+  preserveUnknownFields: false
+  subresources:
+    status: {}
+  validation:
+    openAPIV3Schema:
+      type: object
+      properties:
+        spec:
+          type: object
+          oneOf:
+            - required: ["id","plugins"]
+          properties:
+            id:
+              type: integer
+              minimum: 0
+            labels:
+              type: array

Review comment:
       ok, i will modify this and push right now




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r715500265



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: apisixpluginconfigs
+    singular: apisixpluginconfigs
+    kind: ApisixPluginConfig
+    shortNames:
+      - apc
+  preserveUnknownFields: false
+  subresources:
+    status: {}
+  validation:
+    openAPIV3Schema:
+      type: object
+      properties:
+        spec:
+          type: object
+          oneOf:
+            - required: ["id","plugins"]
+          properties:
+            id:
+              type: integer
+              minimum: 0
+            labels:
+              type: array

Review comment:
       ```suggestion
                 type: object
   ```
   
   The labels should be `object`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927223400


   > The structure of ApisixPluginConfigSpec can be as follows, it only needs to include the following two fields. (you can add some comments)
   > 
   > ```go
   > type ApisixPluginConfigSpec struct {
   > 	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
   > 	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins" yaml:"plugins"`
   > }
   > ```
   
   but the `ApisixRouteHTTPPluginConfig` and `ApisixStatus` don't exist in the new created file `pkg/kube/apisix/apis/config/v1alpha1/types.go`, 
   so should i import them from the `pkg/kube/apisix/apis/config/v2alpha1/types.go`
   @tao12345666333 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716130547



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`

Review comment:
       I copied labels from the [`https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L830`](https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L830) you had provided before. 
   I thought what I should do was just to translate it from lua to go.
   i will delete it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926536613


   > @neverCase you should re-run `make codegen` and push it.
   > 
   > ```
   > make codegen
   > git add pkg/*
   > git commit -m 're-run codegen'
   > git push 
   > ```
   
   should i add `ApisixPluginConfigList` first.
   There some errors in the pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta1/apisixpluginconfig.go
   ```sh
   // List takes label and field selectors, and returns the list of ApisixPluginConfigs that match those selectors.
   func (c *apisixPluginConfigs) List(ctx context.Context, opts v1.ListOptions) (result *v2beta1.ApisixPluginConfigList, err error) {
   	var timeout time.Duration
   	if opts.TimeoutSeconds != nil {
   		timeout = time.Duration(*opts.TimeoutSeconds) * time.Second
   	}
   	result = &v2beta1.ApisixPluginConfigList{}
   	err = c.client.Get().
   		Namespace(c.ns).
   		Resource("apisixpluginconfigs").
   		VersionedParams(&opts, scheme.ParameterCodec).
   		Timeout(timeout).
   		Do(ctx).
   		Into(result)
   	return
   }
   ```
   The structure `ApisixPluginConfigList` maybe be required.
   @tao12345666333 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714486829



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins,omitempty" yaml:"plugins,omitempty"`
+}
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// ApisixPluginConfigList contains a list of ApisixPluginConfig.
+type ApisixPluginConfigList struct {

Review comment:
       do u meaning the `ApisixPluginConfigList`?
   ok, i will delete it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-977834733


   We can use https://github.com/apache/apisix-ingress-controller/pull/694 instead, I will close this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716121775



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`

Review comment:
       We don't need to define ID manually. The `namespace-name` can be used as ID. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927226426


   you can push you change first (before run `make codegen`)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927226144


   i find some problem when generating zz_generated.deepcopy.go
   cause the previous deepcopy files's head was:
   ```sh
   // +build !ignore_autogenerated
   
   // Licensed to the Apache Software Foundation (ASF) under one or more
   // contributor license agreements.  See the NOTICE file distributed with
   // this work for additional information regarding copyright ownership.
   // The ASF licenses this file to You under the Apache License, Version 2.0
   // (the "License"); you may not use this file except in compliance with
   // the License.  You may obtain a copy of the License at
   ...
   ```
   but when i generated on my mac, it would add an another line before:
   ```sh
   //go:build !ignore_autogenerated
   // +build !ignore_autogenerated
   
   // Licensed to the Apache Software Foundation (ASF) under one or more
   // contributor license agreements.  See the NOTICE file distributed with
   // this work for additional information regarding copyright ownership.
   // The ASF licenses this file to You under the Apache License, Version 2.0
   // (the "License"); you may not use this file except in compliance with
   // the License.  You may obtain a copy of the License at
   ....
   ```
   i don't know the reason why the difference `//go:build !ignore_autogenerated` to occur.
   @tao12345666333 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926536613


   > @neverCase you should re-run `make codegen` and push it.
   > 
   > ```
   > make codegen
   > git add pkg/*
   > git commit -m 're-run codegen'
   > git push 
   > ```
   
   should i add `ApisixPluginConfigList` first.
   There some errors in the pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta1/apisixpluginconfig.go
   ```sh
   // List takes label and field selectors, and returns the list of ApisixPluginConfigs that match those selectors.
   func (c *apisixPluginConfigs) List(ctx context.Context, opts v1.ListOptions) (result *v2beta1.ApisixPluginConfigList, err error) {
   	var timeout time.Duration
   	if opts.TimeoutSeconds != nil {
   		timeout = time.Duration(*opts.TimeoutSeconds) * time.Second
   	}
   	result = &v2beta1.ApisixPluginConfigList{}
   	err = c.client.Get().
   		Namespace(c.ns).
   		Resource("apisixpluginconfigs").
   		VersionedParams(&opts, scheme.ParameterCodec).
   		Timeout(timeout).
   		Do(ctx).
   		Into(result)
   	return
   }
   ```
   The structure `ApisixPluginConfigList` maybe be required.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r715611430



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`

Review comment:
       🤔 What's the purpose of using labels here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716121882



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: apisixpluginconfigs
+    singular: apisixpluginconfigs
+    kind: ApisixPluginConfig
+    shortNames:
+      - apc
+  preserveUnknownFields: false
+  subresources:
+    status: {}
+  validation:
+    openAPIV3Schema:
+      type: object
+      properties:
+        spec:
+          type: object
+          oneOf:

Review comment:
       Why `oneOf`? Both are required.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716121113



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`
+	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	// Must be required.

Review comment:
       ```suggestion
   	// +required
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927212290


   IMO this CRD is simple, we could use [kubebuilder markers](https://book.kubebuilder.io/reference/markers.html) to generate schema.
   
   This is a reference use case: https://github.com/apache/apisix-ingress-controller/blob/master/pkg/kube/apisix/apis/config/v1/types.go#L272-L328
   
   After defining markers, run the following command in project root (assuming your are using linux) to regen the schema yaml:
   
   ```bash
   controller-gen schemapatch:manifests=./samples/deploy/crd/v1beta1  output:dir=./samples/deploy/crd/v1beta1 paths=./pkg/kube/apisix/...
   ```
   
   This command will raise some errors because not all CRD definitions works well with controller-gen. So you should rollback all changed files except ApisixPluginConfig one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927212290


   IMO this CRD is simple, we could use [kubebuilder markers](https://book.kubebuilder.io/reference/markers.html) to generate schema.
   
   This is a reference use case: https://github.com/apache/apisix-ingress-controller/blob/master/pkg/kube/apisix/apis/config/v1/types.go#L272-L328
   
   After defining markers, run the following command in project root (assuming your are using linux) to regen the schema yaml:
   
   ```bash
   controller-gen schemapatch:manifests=./samples/deploy/crd/v1beta1  output:dir=./samples/deploy/crd/v1beta1 paths=./pkg/kube/apisix/...
   ```
   
   This command will raise some errors because not all CRD definitions works well with controller-gen. So you should rollback all changed files except fo ApisixPluginConfig one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926562685


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#689](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (49be139) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/3e9bdbf0cee6d49c8e0db27152d46565df704e8c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3e9bdbf) will **decrease** coverage by `1.32%`.
   > The diff coverage is `0.95%`.
   
   > :exclamation: Current head 49be139 differs from pull request most recent head 475c742. Consider uploading reports for the commit 475c742 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #689      +/-   ##
   ==========================================
   - Coverage   35.37%   34.04%   -1.33%     
   ==========================================
     Files          60       61       +1     
     Lines        5967     6144     +177     
   ==========================================
   - Hits         2111     2092      -19     
   - Misses       3612     3801     +189     
   - Partials      244      251       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [pkg/ingress/compare.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvY29tcGFyZS5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/controller.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvY29udHJvbGxlci5nbw==) | `1.06% <0.00%> (-0.01%)` | :arrow_down: |
   | [pkg/ingress/secret.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3Mvc2VjcmV0Lmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/apisix\_ssl.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3NzbC5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | `47.00% <ø> (ø)` | |
   | [pkg/ingress/pod.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvcG9kLmdv) | `32.46% <100.00%> (+0.88%)` | :arrow_up: |
   | [pkg/apisix/plugin.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9wbHVnaW4uZ28=) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `27.25% <0.00%> (-3.50%)` | :arrow_down: |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `35.66% <0.00%> (-2.10%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [3e9bdbf...475c742](https://codecov.io/gh/apache/apisix-ingress-controller/pull/689?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 closed pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 closed pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926531492


   @neverCase  you should re-run `make codegen` and push it.
   
   ```
   make codegen
   git add pkg/*
   git commit -m 're-run codegen'
   git push 
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927223916


   like this:
   
   https://github.com/apache/apisix-ingress-controller/blob/957c31522e1b1e5f8ef9cab7eb244473a4e0f675/pkg/kube/apisix/apis/config/v1/types.go#L23


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r715304710



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,64 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Cluster

Review comment:
       ok, i will change the errors above, then run `make codegen` and open a next pr




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714485988



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins,omitempty" yaml:"plugins,omitempty"`
+}
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// ApisixPluginConfigList contains a list of ApisixPluginConfig.
+type ApisixPluginConfigList struct {

Review comment:
       It can be deleted, no need for it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716121070



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`
+	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	// Must be required.

Review comment:
       We could use `+required` annotation.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927213812


   ApisixPluginConfig is a new resource.
   According to the management method of the Kubernetes API[1], we should start with the v1alpha1 version.
   This means that you should create a `pkg/kube/apisix/apis/config/v1alpha1` directory and move your current content to it.
   
   
   [1]: https://kubernetes.io/docs/concepts/overview/kubernetes-api/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716120025



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`

Review comment:
       Well, Labels do not have to be exposed to users, labels for APISIX objects are used for the management purpose.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926530290


   @tao12345666333 
   ci run failed due to clientset need `ApisixPluginConfigList`, but i had deleted it before.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 edited a comment on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 edited a comment on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927213812


   ApisixPluginConfig is a new resource.
   According to the management method of the [Kubernetes API], we should start with the v1alpha1 version.
   This means that you should create a `pkg/kube/apisix/apis/config/v1alpha1` directory and move your current content to it.
   
   
   [Kubernetes API]: https://kubernetes.io/docs/concepts/overview/kubernetes-api/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927226528


   Submit complete changes and code to make it easier to let us know what happened.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927223742


   @neverCase  yep, you should import them


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716131190



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: apisixpluginconfigs
+    singular: apisixpluginconfigs
+    kind: ApisixPluginConfig
+    shortNames:
+      - apc
+  preserveUnknownFields: false
+  subresources:
+    status: {}
+  validation:
+    openAPIV3Schema:
+      type: object
+      properties:
+        spec:
+          type: object
+          oneOf:

Review comment:
       > The structure of ApisixPluginConfigSpec can be as follows, it only needs to include the following two fields. (you can add some comments)
   > 
   > ```go
   > type ApisixPluginConfigSpec struct {
   > 	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
   > 	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins" yaml:"plugins"`
   > }
   > ```
   
   👌




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716131162



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`
+	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	// Must be required.

Review comment:
       > The structure of ApisixPluginConfigSpec can be as follows, it only needs to include the following two fields. (you can add some comments)
   > 
   > ```go
   > type ApisixPluginConfigSpec struct {
   > 	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
   > 	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins" yaml:"plugins"`
   > }
   > ```
   
   👌




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716128497



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`

Review comment:
       ok, i will delete it




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] neverCase commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
neverCase commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927226144


   i find some problem when generating zz_generated.deepcopy.go
   cause the previous deepcopy files's head was:
   ```sh
   // +build !ignore_autogenerated
   
   // Licensed to the Apache Software Foundation (ASF) under one or more
   // contributor license agreements.  See the NOTICE file distributed with
   // this work for additional information regarding copyright ownership.
   // The ASF licenses this file to You under the Apache License, Version 2.0
   // (the "License"); you may not use this file except in compliance with
   // the License.  You may obtain a copy of the License at
   ...
   ```
   but when i generated on my mac, it would add an another line before:
   ```sh
   //go:build !ignore_autogenerated
   // +build !ignore_autogenerated
   
   // Licensed to the Apache Software Foundation (ASF) under one or more
   // contributor license agreements.  See the NOTICE file distributed with
   // this work for additional information regarding copyright ownership.
   // The ASF licenses this file to You under the Apache License, Version 2.0
   // (the "License"); you may not use this file except in compliance with
   // the License.  You may obtain a copy of the License at
   ....
   ```
   i don't know the reason why the difference `//go:build !ignore_autogenerated` to occur.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-927220621


   The structure of ApisixPluginConfigSpec can be as follows, it only needs to include the following two fields. (you can add some comments)
   
   ```go
   type ApisixPluginConfigSpec struct {
   	Desc   string                   `json:"desc,omitempty" yaml:"desc,omitempty"`
   	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins" yaml:"plugins"`
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714506786



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,64 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: apisixpluginconfigs.apisix.apache.org
+spec:
+  group: apisix.apache.org
+  versions:
+    - name: v2alpha1
+      served: true
+      storage: true
+  scope: Cluster

Review comment:
       ```suggestion
     scope: Namespaced
   ```
   
   We can set it to ns level, so as to avoid interference




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r716187106



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,46 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigLabels was key/value pairs to specify attributes
+// and the maxProperties could only be 16.
+type ApisixPluginConfigLabels map[string]string
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// Id must be required.
+	Id int32 `json:"id" yaml:"id"`
+	// Labels use ApisixPluginConfigLabels
+	Labels ApisixPluginConfigLabels `json:"labels,omitempty" yaml:"labels,omitempty"`

Review comment:
       We may take over the labels for apisix objects, but the users of CRD(s) don't have this because Kubernetes already have the label mechanism.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714502681



##########
File path: samples/deploy/crd/v1beta1/ApisixPluginConfig.yaml
##########
@@ -0,0 +1,64 @@
+#

Review comment:
       No, Currently `make codegen` does not generate CRD




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#issuecomment-926538164


   @neverCase  yes. you should add it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714488820



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+
+// ApisixPluginConfig is the Schema for the ApisixPluginConfig resource.
+// An ApisixPluginConfig is used to support a group of plugin configs
+type ApisixPluginConfig struct {
+	metav1.TypeMeta   `json:",inline" yaml:",inline"`
+	metav1.ObjectMeta `json:"metadata" yaml:"metadata"`
+
+	// Spec defines the desired state of ApisixPluginConfigSpec.
+	Spec   ApisixPluginConfigSpec `json:"spec" yaml:"spec"`
+	Status ApisixStatus           `json:"status,omitempty" yaml:"status,omitempty"`
+}
+
+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`
+	// Plugins contains a list of ApisixRouteHTTPPluginConfig
+	Plugins []ApisixRouteHTTPPluginConfig `json:"plugins,omitempty" yaml:"plugins,omitempty"`
+}
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// ApisixPluginConfigList contains a list of ApisixPluginConfig.
+type ApisixPluginConfigList struct {

Review comment:
       yes




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #689: feat: init ApisixPluginConfig crd (#638)

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #689:
URL: https://github.com/apache/apisix-ingress-controller/pull/689#discussion_r714506395



##########
File path: pkg/kube/apisix/apis/config/v2beta1/types.go
##########
@@ -197,3 +197,35 @@ type ApisixRouteList struct {
 	metav1.ListMeta `json:"metadata" yaml:"metadata"`
 	Items           []ApisixRoute `json:"items,omitempty" yaml:"items,omitempty"`
 }
+
+// +genclient
+// +genclient:nonNamespaced

Review comment:
       Yes, we can set it to ns scope, so as to avoid interference




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org