lukasoppermann / design-tokens

🎨 Figma plugin to export design tokens to json in an amazon style dictionary compatible format.
https://www.figma.com/community/plugin/888356646278934516/Design-Tokens
MIT License
961 stars 134 forks source link

Feature Request: Support Gitlab CI/CD Variables #182

Open dgmartin opened 2 years ago

dgmartin commented 2 years ago

As mentioned in #181, sending a payload up to Gitlab is currently blocked due to the inability to pass the json payload however taking this a step further it would be better to just support Gitlab directly and pass the data directly to Gitlab variables using the POST command documented here. When passing the bulk payload to the webhook call the end user needs to parse out the data within the Gitlab job. By passing this directly into variables via POST this step is skipped and the user can just grab the token json directly. With the increased use of CI/CD pipelines on the rise and Gitlab one of the leaders in the field this improvement would unlock the ability for a a wide range of users.

For example in the readme file it's described that the export data is sent as:

  "event_type": "update-tokens", // or any string you defined
  "client_payload": { 
    "tokens": "{...}", // the stringified json object holding all your desing tokens
    "filename": "Design Tokens" // the Figma file name from which the tokens were exported
  }

However you could pass the same data to the POST form such as:

  --form token=TOKEN
  --form ref=main
  --form "variables[FIGMA_EVENT_TYPE]=update-tokens" // or any string you defined
  --form "variables[FIGMA_CLIENT_PAYLOAD_TOKENS]={...}" // the stringified json object holding all your desing tokens
  --form "variables[FIGMA_CLIENT_PAYLOAD_FILENAME]=Design Tokens" // the Figma file name from which the tokens were exported

Note: In the example above I proceed each variable name with "FIGMA_" to avoid collisions but that and the variable names themselves can be changed to whatever you would like.

lukasoppermann commented 2 years ago

Hey @dgmartin,

I am not using Gitlab so I don't have experience here, but since it is a very common tool I am happy to work with you on improving the support.

I would like to figure out a viable solution in this ticket first. Here is what I am thinking could be a solution, let me know if this is true:

We add an option "send payload as post variables" that the user can toggle. If it is toggled we send the post variables.

However, I am wondering if this is GitLab only?

dgmartin commented 2 years ago

@lukasoppermann I started working on this change but I need to verify a few things and I want to see if I can test this. If you know of any good documentation on how to do this let me know. I will also wait for #181 to be merged in before I put up a PR for this as well. You can see the changes I have so far in this commit log. Let me know if I'm going about this the wrong way or if you see any areas that needs improvement

lukasoppermann commented 2 years ago

Generally looks good.

So on the UI you have a new item in the Dropdown for Auth, right? And if this is Gitlab you will show another new field ref, correct?

dgmartin commented 2 years ago

Currently I'm not hiding the new ref field, but yes that is one of the items that I want to add. I'd like it to be dynamic exactly as you said. Otherwise your description is correct.

lukasoppermann commented 2 years ago

Hey @dgmartin this can be closed as fixed now, correct?

HopperDai commented 2 years ago

Can't find the field of 'ref' and 'token' in payload when use gitlab. Throw error when request the api. @lukasoppermann @dgmartin

image

image

lukasoppermann commented 2 years ago

@dgmartin any ideas?

The issue could be in line 74 of the export file if(exportSettings.authType === "Gitlab_Token") { as the in the settings the key is gitlab_token.

I'll try to fix this.

lukasoppermann commented 2 years ago

Hmm, @dgmartin any ideas? Hey @HopperDai can you please check it again? Fix what I think was the bug and I am seeing now the --form values in the console.

HopperDai commented 2 years ago

Hmm, @dgmartin any ideas? Hey @HopperDai can you please check it again? Fix what I think was the bug and I am seeing now the --form values in the console. @lukasoppermann

I can see values in the payload. But the request is still fail. image image

This is the config. I have also tried multipart/form-data content-type. image

lukasoppermann commented 2 years ago

Hey @HopperDai I don't have a gitlab to test with. We need to get help from @dgmartin here. I am not sure if this is an issue with the plugin or with gitlab setup now.

HopperDai commented 2 years ago

@lukasoppermann I run the command in terminal which is recommend by gitlab. It's succeed. image image

lukasoppermann commented 2 years ago

Hmm, I would assume that the issue is with the way the form data is transferred.

The implementation currently uses the body and adds formdata.

    body = new FormData()
    body.append('token', exportSettings.accessToken)
    body.append('ref', exportSettings.reference)

Do you have any ideas?

dgmartin commented 2 years ago

@lukasoppermann and @HopperDai Sorry for the delayed response. I currently work on this with a colleague and we just got to start looking into this yesterday. We found the original issue with the incorrect token value being checked. I was hoping to confirm this today but it seems @lukasoppermann fix worked in that regards. Thank you @lukasoppermann for getting that fix in.

I'm seeing the same error @HopperDai is seeing. When I test this with postman I'm able to send a similar request that works however I'm seeing two differences that stick out to me.

  1. There is an additional space between the form title and the value when sending via this plugin. I'm not sure what is causing this to add the extra gap or if this is even causing a problem. Based on the values posted (all correct) I feel this is less likely to be the issue but something to keep an eye on.
  2. This issue might actually be a result of setting the Content-Type manually. Postman is sending the content-type as multipart/form-data; boundary=--------------------------<generated number here> however when we set the value manually we are leaving off the boundary. This may causing the server to be unaware of where the key/value pairs begin and end.

I've been very busy at work the last few days but I'm hoping to have some time to look into this later today. My colleague also found a better way for use to test this locally so hopefully we will be able to verify this is working before we push up any changes. My plan is to focus on the second issue and see if we can set the content-type only when it is NOT a gitlab token option. My guess is that XMLHttpRequest will add the form data content type automatically at that point and include the necessary boundary data.

On the matter of XMLHttpRequest, I was highly recommended by another coworker who is better versed in web development that the XMLHttpRequest should be replaced by the newer "fetch" API. This may be another option for a fix however it would be a more extreme change at this point so I'm going to avoid that option for now.

dgmartin commented 2 years ago

@lukasoppermann and @HopperDai I believe I was correct about the Content-Type. I ran some initial tests with my colleague and was able to get this running correctly. I've made some changes and push it up to my branch. Tomorrow I will need to run a final test and I should be able to open the following PR as ready: https://github.com/lukasoppermann/design-tokens/pull/201

HopperDai commented 2 years ago

@dgmartin I debug in local environment with your fixed code. It can run. Thanks!

dgmartin commented 2 years ago

@HopperDai I'm glad it's working for you but unfortunately I have some bittersweet news. While the fix I've implemented does fix the issue it seems there is a bigger issue that my colleague and I have run into. Essentially we are hitting a data limit on the amount of JSON data that we can transfer into the Gitlab pipeline. I can provide further detail on the nature of this error upon request. While my current code might work for a smaller Figma files, larger files risk running into the same error. I am currently working on opening a communication channel directly with Gitlab to see if there is anything we can do about this immediately and if not I have an idea for a feature request that I will submit to Gitlab upon hearing back from them.

This brings me to a decision that we need to make regarding the Gitlab feature on this plugin. The way I see it we have two options:

  1. Add a warning stating that using the Gitlab Token may lead to data caps if the Figma file is too large and keep this functionality in this plugin.
  2. We revert these changes and remove the Gitlab functionality entirely. @lukasoppermann as this is your project this is entirely your decision to make. Personally I feel we should keep this but make sure we document this issue so that developers are aware of the circumstances. It also seems like @HopperDai is able to use what we have so this may prove useful to some. That being said, due to the current limitations and this being an incomplete solution I could fully understand wanting to remove this as well.

@lukasoppermann if you do decide to go with option one, I can make sure to update the documentation to explain the issue. I still need to add unit tests to my PR but once those two items are complete we can get this change merged in. As far as timeline I can probably get most of this done this weekend but I will need to wait to validate my changes until Monday when my colleague is available.

lukasoppermann commented 2 years ago

Hey @dgmartin is this issue one specifically with your setup because you have a big / huge file, or does this happen with small files already?

If this only happens for large files, I would go for 1. as it may still be helpful for teams with gitlab that have smaller files or break their system into multiple files.

You have used the compressed output, correct?

tatashidayat commented 1 year ago

Hi @HopperDai , I use this config and its works image I was getting 401 before due to using a project token. Found out it requires a pipeline token. https://docs.gitlab.com/ee/ci/triggers/index.html#create-a-pipeline-trigger-token

lukasoppermann commented 1 year ago

Hey @tatashidayat happy to accept a PR for this/ Do you think you could add this?