feat: init ApisixPluginConfig crd #4 (#638) #694

neverCase · 2021-09-26T06:40:55Z

Please answer these questions before submitting a pull request

Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
proposal: Add CRD ApisixPluginConfig support plugin configs #638
feat: init ApisixPluginConfig crd (#638) #689

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

neverCase · 2021-09-26T08:29:30Z

@tao12345666333

tao12345666333 · 2021-10-13T06:28:10Z

pkg/kube/apisix/apis/config/v1alpha1/types.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:subresource:status
+// +kubebuilder:object:generate=true
+// +kubebuilder:printcolumn:name="Id",type="date",JSONPath=".metadata.namespace"


the type is incorrect

hi, i'm busy now just after the national vocation -,-.
i will update my code latter.

tao12345666333 · 2021-10-13T06:29:19Z

@neverCase any update?

# Conflicts: # samples/deploy/crd/v1/ApisixPluginConfig.yaml

neverCase · 2021-10-20T09:57:27Z

@tao12345666333 hi, i'm back.

tao12345666333 · 2021-10-20T10:03:58Z

cool!

neverCase · 2021-10-21T02:53:08Z

@tao12345666333
hi, i have some problems with this pr.
From the ci results above, it shows

Error: pkg/kube/apisix/apis/config/v1alpha1/register.go:42: File is not `gofmt`-ed with `-s` (gofmt)
	scheme.AddKnownTypes(SchemeGroupVersion,
		//&ApisixPluginConfig{},
		//&ApisixPluginConfigList{},
	)
Error: Process completed with exit code 1.

I had annotated the &ApisixPluginConfig{} and &ApisixPluginConfigList{} here, because i hadn't generated the zz_generated.deepcopy.go.
Without a series of deepcopy * functions, these two objects couldn't implement the k8s runtime.Object.
so what should i do next

tao12345666333 · 2021-10-30T14:03:21Z

Just run make codegen

neverCase · 2021-11-02T03:05:56Z

@tao12345666333 hi, i had run the make codegen and make lint

neverCase · 2021-11-19T07:11:57Z

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.

I had add client implementation in the early commit.
Now what i need to do is adding ApisixPluginConfig translator

… ApisixPluginConfig crd (apache#638)

codecov-commenter · 2021-11-22T07:41:20Z

Codecov Report

Merging #694 (e1c0e3f) into master (9f2cd7f) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
- Coverage   32.59%   32.58%   -0.01%     
==========================================
  Files          66       65       -1     
  Lines        6780     6779       -1     
==========================================
- Hits         2210     2209       -1     
  Misses       4321     4321              
  Partials      249      249

Impacted Files	Coverage Δ
test/e2e/e2e.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f2cd7f...e1c0e3f. Read the comment docs.

neverCase · 2021-11-22T07:42:13Z

I forgot the license header, lol.

neverCase · 2021-11-22T07:46:39Z

@tao12345666333
plz review it once again

tao12345666333

The CRD api-version should be upgrade.

tao12345666333 · 2021-11-22T07:50:44Z

samples/deploy/crd/v1/ApisixPluginConfig.yaml

+# limitations under the License.
+#
+
+apiVersion: apiextensions.k8s.io/v1beta1


We have upgraded its version to v1, please upgrade it. #693

We have upgraded its version to v1, please upgrade it. #693
ok

tao12345666333 · 2021-11-22T07:56:06Z

In #707, we discussed that we want to use a unified version for all resource management, so here could you add it to the v2beta2 version?

I'm sorry that this is inconsistent with our initial expectations.

neverCase · 2021-11-22T08:02:00Z

Never mind.
I will update right now

neverCase · 2021-11-22T09:17:29Z

@tao12345666333
plz review it

pkg/kube/apisix/apis/config/v2beta2/types.go

tokers · 2021-11-22T11:03:47Z

pkg/kube/apisix/apis/config/v2beta2/types.go

+// ApisixPluginConfigSpec defines the desired state of ApisixPluginConfigSpec.
+type ApisixPluginConfigSpec struct {
+	// +kubebuilder:validation:MinLength=1
+	Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`


Is this necessary? The Desc for the underlying plugin_config object is used for the management purpose, I think it's not useful for users, they can add descriptions from the annotations.

I think we can keep it like this

Is this necessary? The Desc for the underlying plugin_config object is used for the management purpose, I think it's not useful for users, they can add descriptions from the annotations.

I think we can keep it like this.

But we don't have such field for other objects, and so far no users report such demands, so I doubt the necessity of it.

If we look at it from the point of view of consistency with other resources, we really don’t need to keep it.

@neverCase could you please remove this field?

samples/deploy/crd/v1/ApisixPluginConfig.yaml

neverCase · 2021-11-23T02:43:30Z

@tao12345666333
hi, plz review it

samples/deploy/crd/v1/ApisixPluginConfig.yaml

tao12345666333 · 2021-11-23T03:10:44Z

#746 is deleting v1 and v2alpha1, so we don't need to keep these two versions in the CRD definition.

In addition, please fix the lint error. Thanks!

neverCase · 2021-11-23T03:52:47Z

@tao12345666333
I had fixed the lint error about my commits

golangci-lint run
pkg/kube/apisix/apis/config/v2beta2/types.go:211: File is not `gofmt`-ed with `-s` (gofmt)
	Status ApisixStatus  `json:"status,omitempty" yaml:"status,omitempty"`
pkg/apisix/plugin_test.go:25: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"
pkg/apisix/schema_test.go:24: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"

But there were still some errors left:

golangci-lint run
pkg/apisix/plugin_test.go:25: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"
pkg/apisix/schema_test.go:24: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"

Maybe this caused by my latest golangci-lint
golangci-lint has version 1.42.1 built from 54f4301 on 2021-09-04T14:39:09Z
so what could I do to fix it or just ignore it

tokers · 2021-11-23T09:32:35Z

@tao12345666333 I had fixed the lint error about my commits

golangci-lint run
pkg/kube/apisix/apis/config/v2beta2/types.go:211: File is not `gofmt`-ed with `-s` (gofmt)
	Status ApisixStatus  `json:"status,omitempty" yaml:"status,omitempty"`
pkg/apisix/plugin_test.go:25: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"
pkg/apisix/schema_test.go:24: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"

But there were still some errors left:

golangci-lint run
pkg/apisix/plugin_test.go:25: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"
pkg/apisix/schema_test.go:24: File is not `goimports`-ed with -local github.com/apache/apisix-ingress-controller (goimports)
	"github.com/apache/apisix-ingress-controller/pkg/metrics"

Maybe this caused by my latest golangci-lint golangci-lint has version 1.42.1 built from 54f4301 on 2021-09-04T14:39:09Z so what could I do to fix it or just ignore it

If it's reported by the lint tool, let's fix it.

neverCase · 2021-11-24T02:51:01Z

@tao12345666333 @tokers
Now, what could I do next

tao12345666333 · 2021-11-24T06:40:52Z

ping @gxthrj

tao12345666333 · 2021-11-24T10:56:59Z

samples/deploy/crd/v1/ApisixPluginConfig.yaml

+              required:
+                - plugins
+              properties:
+                desc:


And remove this one.

tao12345666333 · 2021-11-24T10:59:03Z

@neverCase There is no problem overall, please delete the desc field and run make codegen again. Then we can merge this PR. Thanks!

neverCase · 2021-11-24T11:26:46Z

@neverCase There is no problem overall, please delete the desc field and run make codegen again. Then we can merge this PR. Thanks!

@tao12345666333
ok, one minute

neverCase · 2021-11-24T11:40:17Z

@tao12345666333
hi，i'm ready.

tao12345666333 · 2021-11-24T12:30:11Z

wait for CI

tao12345666333

Thanks！

feat: init ApisixPluginConfig crd #4 (apache#638)

b0a0764

tao12345666333 reviewed Oct 13, 2021

View reviewed changes

neverCase added 2 commits October 20, 2021 17:53

Merge remote-tracking branch 'origin/master' into feat-638

e3fa013

# Conflicts: # samples/deploy/crd/v1/ApisixPluginConfig.yaml

feat: delete wrong kubebuilder tags #5 (apache#638)

457ba27

feat: annotate scheme.AddKnownTypes temporarily (apache#638)

181b087

neverCase added 2 commits November 2, 2021 10:42

Merge remote-tracking branch 'origin/master' into feat-638

5d0ffb0

feat: generate codes for ApisixPluginConfig crd (apache#638)

00c120d

tao12345666333 mentioned this pull request Nov 19, 2021

proposal: Add CRD ApisixPluginConfig support plugin configs #638

Closed

6 tasks

neverCase added 2 commits November 22, 2021 11:47

Merge remote-tracking branch 'origin/master' into feat-638

c2ddf0f

feat: add rbac and e2e for ApisixPluginConfig crd (apache#638)

e84d7d4

neverCase marked this pull request as ready for review November 22, 2021 07:30

tao12345666333 requested review from tokers, gxthrj, tao12345666333 and lingsamuel November 22, 2021 07:33

feat: error CustomResourceDefinition and missing license header about…

b34b001

… ApisixPluginConfig crd (apache#638)

tao12345666333 requested changes Nov 22, 2021

View reviewed changes

refactor ApisixPluginConfig from v1alpha1 to v2beta2

bcee7fc

refactor ApisixPluginConfig from v1alpha1 to v2beta2 (apache#638)

556ae91

tokers reviewed Nov 22, 2021

View reviewed changes

remove deprecated ApisixPluginConfig v1alpha1 files (apache#638)

e1c0e3f

tao12345666333 reviewed Nov 23, 2021

View reviewed changes

samples/deploy/crd/v1/ApisixPluginConfig.yaml Outdated Show resolved Hide resolved

samples/deploy/crd/v1/ApisixPluginConfig.yaml Outdated Show resolved Hide resolved

fix golingci-lint error (apache#638)

92c6d9c

tao12345666333 reviewed Nov 24, 2021

View reviewed changes

samples/deploy/crd/v1/ApisixPluginConfig.yaml Outdated

required:

- plugins

properties:

desc:

Copy link

Member

tao12345666333 Nov 24, 2021

Choose a reason for hiding this comment

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

And remove this one.

neverCase added 2 commits November 24, 2021 19:33

Merge remote-tracking branch 'origin/master' into feat-638

9e440b7

feat: remove deprecated field Desc of ApisixPluginConfig (apache#638)

0551789

tokers approved these changes Nov 24, 2021

View reviewed changes

tao12345666333 mentioned this pull request Nov 24, 2021

feat: init ApisixPluginConfig crd (#638) #689

Closed

4 tasks

tao12345666333 approved these changes Nov 24, 2021

View reviewed changes

tao12345666333 merged commit b127ff4 into apache:master Nov 24, 2021

neverCase deleted the feat-638 branch November 25, 2021 02:57

Sindweller pushed a commit to Sindweller/apisix-ingress-controller that referenced this pull request Nov 25, 2021

feat: init ApisixPluginConfig crd apache#4 (apache#638) (apache#694)

6628406

tao12345666333 linked an issue Dec 8, 2021 that may be closed by this pull request

proposal: Add CRD ApisixPluginConfig support plugin configs #638

Closed

6 tasks

feat: init ApisixPluginConfig crd #4 (#638) #694

feat: init ApisixPluginConfig crd #4 (#638) #694

Conversation

neverCase commented Sep 26, 2021 • edited Loading

Bugfix

New feature or improvement

Backport patches

neverCase commented Sep 26, 2021

Choose a reason for hiding this comment

neverCase Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

tao12345666333 commented Oct 13, 2021

neverCase commented Oct 20, 2021

tao12345666333 commented Oct 20, 2021

neverCase commented Oct 21, 2021

tao12345666333 commented Oct 30, 2021

neverCase commented Nov 2, 2021

neverCase commented Nov 19, 2021 • edited Loading

codecov-commenter commented Nov 22, 2021 • edited Loading

Codecov Report

neverCase commented Nov 22, 2021 • edited Loading

neverCase commented Nov 22, 2021

tao12345666333 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tao12345666333 commented Nov 22, 2021

neverCase commented Nov 22, 2021

neverCase commented Nov 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverCase commented Nov 23, 2021

tao12345666333 commented Nov 23, 2021

neverCase commented Nov 23, 2021

tokers commented Nov 23, 2021

neverCase commented Nov 24, 2021

tao12345666333 commented Nov 24, 2021

Choose a reason for hiding this comment

tao12345666333 commented Nov 24, 2021

neverCase commented Nov 24, 2021 • edited Loading

neverCase commented Nov 24, 2021 • edited Loading

tao12345666333 commented Nov 24, 2021

tao12345666333 left a comment

Choose a reason for hiding this comment

neverCase commented Sep 26, 2021 •

edited

Loading

neverCase Oct 13, 2021 •

edited

Loading

neverCase commented Nov 19, 2021 •

edited

Loading

codecov-commenter commented Nov 22, 2021 •

edited

Loading

neverCase commented Nov 22, 2021 •

edited

Loading

neverCase commented Nov 24, 2021 •

edited

Loading

neverCase commented Nov 24, 2021 •

edited

Loading