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

Keyword preservation does not preserve "identifier" fields #770

Closed
6 tasks done
dkobia opened this issue Mar 20, 2023 · 6 comments
Closed
6 tasks done

Keyword preservation does not preserve "identifier" fields #770

dkobia opened this issue Mar 20, 2023 · 6 comments
Labels

Comments

@dkobia
Copy link

dkobia commented Mar 20, 2023

Checklist

  • I have looked into the README and have not found a suitable solution or answer.
  • I have looked into the documentation and have not found a suitable solution or answer.
  • I have searched the issues and have not found a suitable solution or answer.
  • I have upgraded to the latest version of this tool and the issue still persists.
  • I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • I agree to the terms within the Auth0 Code of Conduct.

Description

The new keyword replacement works as required (mostly) but skips a few parameters.

For example, the following will not be preserved in tenant.yml

clientGrants:
  - audience: '@@AUTH0_AUDIENCE@@'
themes:
  - fonts:
      font_url: '@@FONT1_URL@@'
themes:
  - widget:
      logo_url: '@@LOGO_URL@@'

The same is true for any keyword mappings in branding_templates/universal_login.html

Expectation

I expect the AUTH0_KEYWORD_REPLACEMENT_MAPPINGS to be maintained.

Reproduction

  1. Export tenant settings that include changes to the universal login template
  2. Update keyword mappings in the html file
  3. Run the import again

Deploy CLI version

v7.17.0

Node version

v16.10.0

@dkobia dkobia added the bug label Mar 20, 2023
@willvedd
Copy link
Contributor

@dkobia thank you very much for the feedback! I'm glad that this feature is getting a bit of use. I believe that there are two separate issues at play here.

First is that there probably are a couple of fields that the algorithm is not yet able to preserve, especially when they are fields nested underneath arrays (ex: themes). This is because array items cannot be identified by order alone (for technical reasons explained in #688). This is only my initial suspicion and I'll need to investigate further to definitively identify the specific issue.

Secondly, there are fields that just cannot be preserved, particularly the "identifier" field for each resource. The identifier is the field that is used to unique identify a specific resource in the absence of regular IDs to enable tenant-agnostic functionality. I believe that is what is at play with the audience field not being preserved; though I think that the algorithm could be enhanced a bit. Again, I'll need to investigate your specific issues and get back to you.

And finally, It's probably not much consolation, but the keyword preservation is at odds with some technical limitations that prohibit it from operating in 100% of situations but I genuinely hope that it reduces some of the toil.

@sato11
Copy link

sato11 commented Mar 30, 2023

We are facing the same issue but in directory format. Is it possible to change the title so both formats are contained? It wasn't easy for me to relate this to our issue immediately and I was about to submit duplication 😅

Also it would be great if the limitation is documented. Greater if it comes along with the list of "identifier" per resource. Do you think it's possible?

That said, thanks for the feature as well as the maintenance. The new option did reduce some of our toils, about which I wanted to appreciate here 🙏

@willvedd willvedd changed the title Preserving Keyword Replacement Skips Some YAML Parameters Keyword preservation does not preserve "identifier" fields Apr 18, 2023
@willvedd
Copy link
Contributor

@sato11 done. I recognize now that this affects both formats.

I'm working on getting these identifier fields added to the diff'ing algorithm. I believe this can be fixed in code. It still won't work in 100% of cases but will certainly bridge the gap for many folks.

@willvedd
Copy link
Contributor

@dkobia @sato11 I've released an update in 7.17.2 that enables keyword preservation of identifier fields. This will hopefully solve some of the missed replacements that you're observing, but not all. The documented limitations will still apply; these are not likely to be solved anytime soon.

I'll continue to keep this issue open for a little while to solicit feedback.

@bobjohnson2708
Copy link

Hey @willvedd,
Could you publish 7.17.2 to the npm feed? Currently 7.17.1, published 24 days ago is the latest.
We also have a need to preserve identifiers (because of differing identifiers over distinct environments, due to manual creation initially).

@willvedd
Copy link
Contributor

@bobjohnson2708 Apologies, that build got hung up in CI. It's now live on npm.

@willvedd willvedd closed this as completed Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants