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

Unable to construct the decryptor due to lack of decryption password #314

Closed
ddobrin opened this issue May 27, 2019 · 14 comments
Closed

Unable to construct the decryptor due to lack of decryption password #314

ddobrin opened this issue May 27, 2019 · 14 comments
Assignees

Comments

@ddobrin
Copy link
Contributor

ddobrin commented May 27, 2019

The codegen sets the decryptor in the config.json:
decryptorClass: ${config.decryptorClass:com.networknt.decrypt.AESDecryptor}

The service skeleton, after build, fails to start.

Resetting it to:
decryptorClass: com.networknt.decrypt.AESDecryptor, works.

This change is backwards incompatible with existing API code generation.
Is it documented somewhere, as I see an PR was addressed in the last release?

@stevehu
Copy link
Contributor

stevehu commented May 27, 2019

Someone has reported this issue right after the 1.6.2 release. I debugged into it and found that the issue is the parameterization updates all config files unless it is in the exclude list of the config.json file. The workaround is to update the existing config.json to add config.yml into the exclude list; however, this is only a temp solution. I have listed this issue as part of #307

@jiachen1120
Copy link
Contributor

@ddobrin @stevehu Hey!

I think the problem lies in codegen's own configuration file config.json. Because config.yml in the framework is the configuration file of the configuration module, its function is to control the injection of parameters, and it should not be an injectable mode itself. So if we set config.json to the following:

"generateEnvVars": {
    "generate": true,
    "skipArray": true,
    "skipMap": true,
    "exclude": [
      "handler.yml",
      "values.yml"
    ]
  }

Then the generated config.yml will be like this,

decryptorClass: ${config.decryptorClass:com.networknt.decrypt.AESDecryptor}

this is not correct, so we need to update the config.json to exclude config.yml from being a injection mode:

"generateEnvVars": {
    "generate": true,
    "skipArray": true,
    "skipMap": true,
    "exclude": [
      "handler.yml",
      "values.yml",
      "config.yml"
    ]
  }

The config.yml generated in this way is correct.

decryptorClass: com.networknt.decrypt.AESDecryptor

If you agree with this, I will document it soon. Thanks

@ddobrin
Copy link
Contributor Author

ddobrin commented May 29, 2019

Hi @jiachen1120 :
this change is not backwards compatible and, although I have made the change at my client's site, it requires changes across all config.json files and all APIs.

The documentation for this change is missing, and it was not added to the release notes either at the time of the release of this change.

You can document it as such, while clearly indicating that the codegen is not backwards compatible, until we are rationalizing the light-codegen a bit.

This can be done in a couple of steps

@jiachen1120
Copy link
Contributor

jiachen1120 commented May 29, 2019

@ddobrin Yes, I agree. I think the config.yml should always be excluded from injecting parameters. Since once config.yml is not excluded, the generated project would fail to start. To make it backward compatible, maybe we should let config.yml to be ignored from "generateEnvVars" by default. Therefore, users do not need to added it manually. What do you think?

@stevehu
Copy link
Contributor

stevehu commented May 29, 2019

@jiachen1120 We can do it for config.yml but there might be other files in the future. The problem with the exclude list is that there is no way we can make it backward compatible with new config files added to the system. At this stage, I am thinking to remove the parameterization and move the config files into the template so that we can control it very easily.

@jiachen1120
Copy link
Contributor

jiachen1120 commented May 29, 2019

@stevehu Hello, I am not very understanding to remove the parameterization and move the config files into the template so that we can control it very easily. Could you please clarify it?
Currently, the config.yml is built by template which do not include ${}. But the "generateEnvVars" inside config.json (for codegen) would overwrite the generated result with ${} pattern.

@ddobrin
Copy link
Contributor Author

ddobrin commented May 29, 2019

@jiachen: I see this as a series of small steps:

  1. we remove the handlerconfig and make templates out of them; this will allow us to handle them uniformly

  2. rather than parameterizing every file, we can take the approach of parameterizing the files which developers wish to parameterize, and by this the exclusion list would not be required. Parameterization is very useful, and my client uses it a lot, we just need to make it easier for them to work with

  3. Paired with the hot reloading, teams can decide to have 2 buckets of param and no-param, and thus we can avoid an issue when the codegen won't work until somebody adds (in this example config,yml) to the exclusion list. This is just an example

I am looking at the first one for my client (they have customized some of the files, as they have some defaults working well for them) and will update all of you with client fedback

@jiachen1120
Copy link
Contributor

@ddobrin: Agreed. After having a discussion with Steve, a possible solution would be:

  1. same as your first suggestion.
  2. make all the templates be parameterized, without using a config to enable or disable them. Since If we keep using the config.json (codegen), every time we introduce a new config file in framework. There would be some changes in the codegen config, which would lead to a couple between light-4j and light-codegen and backward incompatible.
  3. For users, If they want some customized config file, they can provide their own templates. We can provide a way to help them convert their templates into parameterized config by using commend-line.
    For example, a new tag for codegen-cli, -p / --parameterize customizedConfigName.

Look forward to your client feedback, then, we can RFC it.

@ddobrin
Copy link
Contributor Author

ddobrin commented May 30, 2019

@jiachen1120 @stevehu : Parameterization by individual file name is not going to be sufficient; we must support also one for a folder, where we allow people to specify "param everything in this folder" or separately allow them to provide a list of "param the entire list from here".

this, in addition to what you mention as -p .

@jiachen1120
Copy link
Contributor

@ddobrin Yes, I think to provide a folder path after the -p make more sense! Thanks

@stevehu
Copy link
Contributor

stevehu commented Jul 1, 2019

@stevehu stevehu closed this as completed Jul 1, 2019
@nfacciolo
Copy link

Maybe you forget to add fixes #314 in the release 1.6.5 ? https://github.com/networknt/light-codegen/releases
I have this problem with le released version 1.6.5 following the tutorial here : https://doc.networknt.com/tool/light-codegen/openapi-generator/

@stevehu
Copy link
Contributor

stevehu commented Jul 30, 2019

@NicolasFCO #314 is in the 1.6.5 release; however, the default branch is not updated after the release. We need to think about how to provide the command line jar files to the end-users as we have multiple versions released at the same time. Let's have a chat offline.

@stevehu
Copy link
Contributor

stevehu commented Jul 30, 2019

The source code for the release is attached in each release. You can download and unzip into the networknt workspace and compile it. I am trying to find a way to automatically upload the final jar file to our portal site so that you can download it from there. Also, you can try the light-codegen website to see if it works for you. https://codegen.lightapi.net/form/codeGenSingleForm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants