Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

proposal: Add CRD ApisixPluginConfig support plugin configs #638

Closed
6 tasks done
shelltea opened this issue Aug 18, 2021 · 46 comments · Fixed by #694, #772 or #815
Closed
6 tasks done

proposal: Add CRD ApisixPluginConfig support plugin configs #638

shelltea opened this issue Aug 18, 2021 · 46 comments · Fixed by #694, #772 or #815
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@shelltea
Copy link

shelltea commented Aug 18, 2021

Add a CRD like ApisixPluginConfig to support a group of plugin configs, and ApisixRoute add a field be associated with it.

If we want to implement this function, we can split the following tasks:

@tao12345666333 tao12345666333 added the enhancement New feature or request label Aug 18, 2021
@tao12345666333 tao12345666333 added this to the 1.3.0 milestone Aug 18, 2021
@tao12345666333
Copy link
Member

Add this to v1.3 milestone.

@tao12345666333
Copy link
Member

@tao12345666333
Copy link
Member

cc @Donghui0

@neverCase
Copy link
Contributor

i'm a volunteer

@gxthrj
Copy link
Contributor

gxthrj commented Sep 22, 2021

i'm a volunteer

Great, assign to you. Look forward.

@tao12345666333
Copy link
Member

Thanks! @neverCase If you need help, you can ping me at any time.

@neverCase
Copy link
Contributor

ok, got it

@tokers
Copy link
Contributor

tokers commented Sep 22, 2021

ok, got it

Welcome to submit your plan to the mail list :).

@neverCase
Copy link
Contributor

ok, got it

Welcome to submit your plan to the mail list :).

OK, i will submit my plan later.

@neverCase
Copy link
Contributor

neverCase commented Sep 22, 2021

I had replied a mail on the apache mail list, but it seems something wrong.
After sending, i can't find my reply on the mail list.
@tao12345666333 @tokers

@neverCase
Copy link
Contributor

my plan

  1. add ApisixPluginConfig in types.go and generated libraries (such as clientset)
  2. add ApisixRoute as a group in config
  3. add looper for watching specific resources by using clientset (like k8s.io/client-go) and convert it

@tokers
Copy link
Contributor

tokers commented Sep 23, 2021

I had replied a mail on the apache mail list, but it seems something wrong.
After sending, i can't find my reply on the mail list.
@tao12345666333 @tokers

You have to subscribe the dev@apisix.apache.org before you send any mails.

@tokers
Copy link
Contributor

tokers commented Sep 23, 2021

my plan

  1. add ApisixPluginConfig in types.go and generated libraries (such as clientset)
  2. add ApisixRoute as a group in config
  3. add looper for watching specific resources by using clientset (like k8s.io/client-go) and convert it
  1. Refer to other types (see https://github.com/apache/apisix-ingress-controller/blob/master/pkg/kube/apisix/apis/config/v2beta1/types.go#L30) so that the CRD definition can be generated with the OpenAPIV3Schema.
  2. We need to talk about the mutual exclusivity between pluginConfig and plugins fields. @tao12345666333 @gxthrj @neverCase

@tao12345666333
Copy link
Member

Add a CRD like ApisixPluginConfig to support a group of plugin configs, and ApisixRoute add a field be associated with it.

If we want to implement this function, we can split the following tasks:

  • Add ApisixPluginConfig custom resource. Only need to modify YAML.
  • Add ApisixPluginConfig data structures.
  • Add client implementation to interact with Apache APISIX.
  • Add ApisixPluginConfig translator, it can convert ApisixPluginConfig data structure into APISIX.
  • Add ApisixPluginConfig controller loop, It watches the ApisixPluginConfig resource in the Kubernetes cluster and converts it to the ApisixPluginConfig data structure.
  • Add docs to describe it.

You can split this function into multiple small tasks to carry out, refer to these tasks I added in the description. @neverCase

@tao12345666333
Copy link
Member

I had replied a mail on the apache mail list, but it seems something wrong.
After sending, i can't find my reply on the mail list.
@tao12345666333 @tokers

Yes, you need to subscribe the dev@apisix.apache.org before you send any mails.

Of course, whether you are discussing in this issue or on the mailing list, it’s okay.

@tao12345666333
Copy link
Member

After discussing it with @tokers and @gxthrj . We finally reached an agreement.

We can set the plugins field and pluginconfig field in ApisixRoute to be mutually exclusive.

This means that the user can only configure one of the plugins or pluginconfig in ApisixRoute.

@neverCase
Copy link
Contributor

neverCase commented Sep 23, 2021

ok, i had created ApisixPluginConfig.yaml and add 3 structs ApisixPluginConfig, ApisixPluginConfigSpec, ApisixPluginConfigList in the v2alpha1/types.go.
But i had trouble in making up fields inside ApisixPluginConfigSpec, because i can't find any structs which could be linked to the plugin in types.go.

// +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 {
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// ApisixPluginConfigList contains a list of ApisixPluginConfig.
type ApisixPluginConfigList struct {
	metav1.TypeMeta `json:",inline" yaml:",inline"`
	metav1.ListMeta `json:"metadata" yaml:"metadata"`
	Items           []ApisixPluginConfig `json:"items,omitempty" yaml:"items,omitempty"`
}

could u provide me more details about the plugins
@tao12345666333

@tao12345666333
Copy link
Member

tao12345666333 commented Sep 23, 2021

You can open a PR and submit your current changes directly.

The PluginConfig document is here https://apisix.apache.org/docs/apisix/architecture-design/plugin-config

It looks like this:

// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
type ApisixPluginConfigSpec struct {
    Desc   string            `json:"desc,omitempty" yaml:"desc,omitempty"`
    Plugins []ApisixRouteHTTPPlugin // https://github.com/apache/apisix-ingress-controller/blob/3e9bdbf0cee6d49c8e0db27152d46565df704e8c/pkg/kube/apisix/apis/config/v2beta1/types.go#L118
}

@tao12345666333
Copy link
Member

@neverCase You can use []ApisixRouteHTTPPlugin directly, no need to define the list yourself.

@neverCase
Copy link
Contributor

@neverCase You can use []ApisixRouteHTTPPlugin directly, no need to define the list yourself.

before pr, should i make up the field validation in the ApisixPluginConfig.yaml

@tao12345666333
Copy link
Member

tao12345666333 commented Sep 23, 2021

You can open a PR and set it as draft status. Related discussions can be held in the PR, which will be more convenient.

neverCase added a commit to neverCase/apisix-ingress-controller that referenced this issue Sep 23, 2021
@tao12345666333 tao12345666333 linked a pull request Sep 23, 2021 that will close this issue
4 tasks
neverCase added a commit to neverCase/apisix-ingress-controller that referenced this issue Dec 1, 2021
neverCase added a commit to neverCase/apisix-ingress-controller that referenced this issue Dec 2, 2021
@tao12345666333 tao12345666333 removed a link to a pull request Dec 8, 2021
4 tasks
@tao12345666333
Copy link
Member

@neverCase Thanks for your work, if you have time, you can continue the development of follow-up tasks.

@neverCase
Copy link
Contributor

@neverCase Thanks for your work, if you have time, you can continue the development of follow-up tasks.

ok, got it.
I will update it tomorrow.

@tao12345666333
Copy link
Member

Hi folks,

The main part of the ApisixPluginConfig function has been implemented, thanks for your hard work @neverCase , great!

I’m about to initiate a vote for the release of a new version of v1.4. This feature will appear for the first time in v1.4.

@tao12345666333 tao12345666333 linked a pull request Dec 29, 2021 that will close this issue
8 tasks
@tao12345666333
Copy link
Member

For this feature, there is one last task left.

  • Add docs to describe it.

Who has time to help with this task?

@neverCase
Copy link
Contributor

[ ] Add docs to describe it.

I think it's my job -,-.

@tao12345666333
Copy link
Member

Thanks! @neverCase

neverCase added a commit to neverCase/apisix-ingress-controller that referenced this issue May 18, 2022
@neverCase
Copy link
Contributor

@tao12345666333
This issue could be closed.

@tao12345666333
Copy link
Member

yes, thanks @neverCase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment