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

Sackmesser - Deploy always fails if config.json is present #533

Open
vitor-a-pinto-alb opened this issue May 2, 2022 · 7 comments
Open
Labels
enhancement New feature or request

Comments

@vitor-a-pinto-alb
Copy link

vitor-a-pinto-alb commented May 2, 2022

If config.json (supported by apigee-deploy-maven-plugin config.json ) is present, using sackmesser to deploy the proxy with
sackmesser deploy --googleapi -d <path to proxy> -t $(gcloud auth print-access-token) -e $ENV -o $ORG
Fails with the error
[ERROR] Failed to execute goal io.apigee.build-tools.enterprise4g:apigee-edge-maven-plugin:2.3.1:configure (configure-bundle-step) on project apigee-deployer: null: MojoExecutionException -> [Help 1]

If I remove config.json everything works just fine. This feature of the maven plugin should work when using sackmesser

Attached proxy to reproduce the issue.
mockapiSack.zip

@danistrebel
Copy link
Collaborator

danistrebel commented May 2, 2022

Hi @vitor-a-pinto, can you confirm you're using the config.json you linked? Because the linked file is the Apigee Edge config.json file and some of the resources might not be 100% compatible with the X/hybrid API you target with --googleapi.

@vitor-a-pinto-alb
Copy link
Author

vitor-a-pinto-alb commented May 2, 2022

hi @danistrebel the config.json i'm using is on the zip file. The one linked in the 1st sentence is just for context (still I updated the link to avoid confusion).
Moreover if I try to use maven plugin to deploy the proxy with the config.json on the zip file, with works fine.

@danistrebel
Copy link
Collaborator

Got it thanks, I'll take a look to see what's missing.

@danistrebel
Copy link
Collaborator

Thanks for bringing this up @vitor-a-pinto! He's where I am currently at:

Apigee Sackmesser's intention was to remove the need for writing a manifest and the complexity that comes with Maven. So far this has worked out nicely but it breaks a few features that are dependent on some maven constructs. The config.json is one of them because it depends on a match of the key in the config.json and the profile used. Because the pom.xml is auto generated the profile functionality is not used.

I see the following options in this trade off between flexibility and simplicity:

  1. Sackmesser does not support the config.json functionality and instead encourages templating through external templating or envsubst as we do for the Auth schemes reference.
  2. We provide a default profile in the generated pom and allow for a config.json that holds the config for exactly that profile.
  3. We provide the option to fully customize the generated maven profiles by re-introducing the maven concept of profile.

@OmidTahouri @seymen what do you think?

@vitor-a-pinto-alb
Copy link
Author

vitor-a-pinto-alb commented May 3, 2022

Hi @danistrebel

Just a quick note on your 2nd option: the idea behind the config.json is to be able to customize configs per profile. Speaking of experience, profiles usually match with the different environments (e.g. dev, quality assurance, production, etc). So to have a single profile (the default) wont address the issue completely. Just my 2 cents

@danistrebel
Copy link
Collaborator

Yeah I fully agree. Let me elaborate on 2 a bit more. The motivation would be to make it such that you could have a config-test.json and config-prod.json that you could use as is without having to template. Basically say sackmesser deploy ... --config config-prod.json this way we would retain the configuration flexibility but not leak the maven profiles.

Alternatively (maybe an in-between 2&3) we could do sackmesser deploy ... --config prod where prod picks up the config for prod from the config.json if it exists.

I am not yet convinced this is a great idea so any input is very welcome.

@vitor-a-pinto-alb
Copy link
Author

Ok got it, thanks for your detailed explanation.

@danistrebel danistrebel added the enhancement New feature or request label Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants