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 2022/07/25 09:30:28 UTC
[GitHub] [apisix-dashboard] bzp2010 opened a new pull request, #2540: refactor: config
bzp2010 opened a new pull request, #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540
Please answer these questions before submitting a pull request, **or your PR will get closed**.
**Why submit this pull request?**
- [ ] Bugfix
- [x] New feature provided
- [x] Improve performance
- [ ] Backport patches
**What changes will this PR take into?**
Refactor configuration module.
--
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-dashboard] bzp2010 commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1214877862
ping @nic-chen @starsz @LiteSun
--
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-dashboard] starsz commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
starsz commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1197542694
> In my opinion, PR should not be merged without CI passed. Either do compatibility first, or remove the code first and add them back part by part.
Agree +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-dashboard] bzp2010 commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1196261551
### Note that the code in the current branch will not pass compilation and CI is temporarily closed. When we finish all break updating, we will focus on the build and testing issues.
--
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-dashboard] codecov-commenter commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1212741872
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/2540?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 [#2540](https://codecov.io/gh/apache/apisix-dashboard/pull/2540?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (74c07b0) into [next](https://codecov.io/gh/apache/apisix-dashboard/commit/e68982a6334d369ffdfe4776ec438108462da250?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e68982a) will **decrease** coverage by `1.31%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## next #2540 +/- ##
==========================================
- Coverage 67.33% 66.01% -1.32%
==========================================
Files 194 38 -156
Lines 7433 974 -6459
Branches 864 266 -598
==========================================
- Hits 5005 643 -4362
+ Misses 2107 331 -1776
+ Partials 321 0 -321
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `?` | |
| frontend-e2e-test | `66.01% <ø> (-1.84%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/2540?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../src/pages/User/components/LoginMethodPassword.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9wYWdlcy9Vc2VyL2NvbXBvbmVudHMvTG9naW5NZXRob2RQYXNzd29yZC50c3g=) | `15.38% <0.00%> (-73.08%)` | :arrow_down: |
| [web/src/components/RightContent/AvatarDropdown.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL1JpZ2h0Q29udGVudC9BdmF0YXJEcm9wZG93bi50c3g=) | `50.00% <0.00%> (-32.15%)` | :arrow_down: |
| [web/src/helpers.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9oZWxwZXJzLnRzeA==) | `37.70% <0.00%> (-26.23%)` | :arrow_down: |
| [web/src/components/PanelSection/index.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL1BhbmVsU2VjdGlvbi9pbmRleC50c3g=) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [web/src/pages/Consumer/List.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9wYWdlcy9Db25zdW1lci9MaXN0LnRzeA==) | `69.44% <0.00%> (-22.23%)` | :arrow_down: |
| [web/src/components/Plugin/PluginPage.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9QbHVnaW5QYWdlLnRzeA==) | `82.35% <0.00%> (-16.48%)` | :arrow_down: |
| [web/src/components/Plugin/PluginDetail.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9QbHVnaW5EZXRhaWwudHN4) | `63.68% <0.00%> (-11.18%)` | :arrow_down: |
| [web/src/components/RawDataEditor/RawDataEditor.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL1Jhd0RhdGFFZGl0b3IvUmF3RGF0YUVkaXRvci50c3g=) | `28.57% <0.00%> (-10.39%)` | :arrow_down: |
| [web/src/pages/Consumer/service.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9wYWdlcy9Db25zdW1lci9zZXJ2aWNlLnRz) | `80.00% <0.00%> (-10.00%)` | :arrow_down: |
| [web/src/components/ActionBar/ActionBar.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/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-d2ViL3NyYy9jb21wb25lbnRzL0FjdGlvbkJhci9BY3Rpb25CYXIudHN4) | `69.23% <0.00%> (-7.70%)` | :arrow_down: |
| ... and [161 more](https://codecov.io/gh/apache/apisix-dashboard/pull/2540/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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-dashboard] nic-chen commented on a diff in pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
nic-chen commented on code in PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#discussion_r945749391
##########
api/internal/config/config.go:
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+package config
+
+import (
+ "encoding/json"
+ "fmt"
+ "io/ioutil"
+ "os"
+ "strings"
+
+ "github.com/pkg/errors"
+ "github.com/spf13/viper"
+ "github.com/tidwall/gjson"
+)
+
+func NewDefaultConfig() Config {
+ return Config{
+ Server: Server{
+ HTTPListen: ":9000",
+ HTTPSListen: ":9001",
+ },
+ DataSource: []DataSource{
+ {
+ Name: "default",
+ Type: DataSourceTypeETCD,
+ ETCD: DataSourceETCD{
+ Endpoints: []string{"127.0.0.1:2379"},
+ Username: "",
+ Password: "",
+ MTLS: DataSourceETCDMTLS{},
+ Prefix: "/apisix",
+ },
+ },
+ /*{
+ Name: "default",
+ Type: DataSourceTypeAPISIX,
+ APISIX: DataSourceAPISIX{
+ Address: "http://127.0.0.1:9080",
+ KeyFile: "edd1c9f034335f136f87ad84b625c8f1",
+ },
+ },*/
+ },
+ Security: Security{
+ AllowList: []string{"127.0.0.1", "::1"},
+ CORS: SecurityCORS{
+ AllowCredentials: true,
+ AllowOrigin: "http://dashboard.apisix.local",
+ AllowMethods: "*",
+ AllowHeaders: "Authorization",
+ },
+ XFrameOptions: "deny",
+ ContentSecurityPolicy: "default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'",
+ },
+ Log: Log{
+ ErrorLog: LogError{
+ Level: "warn",
+ FilePath: "logs/error.log",
+ },
+ AccessLog: "logs/access.log",
+ },
+ Authentication: Authentication{
+ ExpireTime: 3600,
+ Users: []AuthenticationUser{
+ {
+ Username: "admin",
+ Password: "admin",
+ },
+ {
+ Username: "user",
+ Password: "user",
+ },
+ },
+ },
+ }
+}
+
+func SetupConfig(c *Config, file string) error {
+ // setup config file path
+ viper.SetConfigFile(file)
+
+ // setup env config search
+ viper.SetEnvPrefix("AD")
+ viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+ viper.AutomaticEnv()
+
+ // load config
Review Comment:
we should add a schema to check the config, could be done in the 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-dashboard] bzp2010 commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1214877509
### Update
It currently completes the refactoring of the `config` package without touching any meaningful CI tests, and eliminates all other package coupling dependencies on `conf`, and currently it completely uses parameter passing to pass configuration to dependencies.
However, it should be noted that part of the CI related to release packaging is turned off and it will be turned on when the v3 version on the `next` branch is ready for release. Also the current configuration pass has been modified so that some of the function arguments have been changed, they are crude and only meet the criteria of being usable, not elegant, I need to optimize them further in iterations, please don't get hung up on the function argument changes.
### Tips
The core of this PR is to remove the `conf` package, implement the new `config` package, strip and replace its use cases. I had to modify a lot of coupled code and related test CIs, such as CLI and E2E. So this PR was huge, involving many files and hundreds of lines of code changes. You can focus on those new feature implementations rather than simply replacing code with packages.
--
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-dashboard] bzp2010 merged pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
bzp2010 merged PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540
--
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-dashboard] LiteSun commented on pull request #2540: refactor: config
Posted by GitBox <gi...@apache.org>.
LiteSun commented on PR #2540:
URL: https://github.com/apache/apisix-dashboard/pull/2540#issuecomment-1197560834
> In my opinion, PR should not be merged without CI passed. Either do compatibility first, or remove the code first and add them back part by part.
+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