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

openapi2jsonschema: Allow writting to subdirectories #276

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peschmae
Copy link

@peschmae peschmae commented Jul 4, 2024

This pull requests extends the openapi2jsonschema.py allowing to write to a subdirectory of the current working directory.

The filename templated from FILENAME_FORMAT is verified to be a child of the working directory, and if the subdirectory doesn't exist yet, it's created.

Verify that the target filename (templates from `FILENAME_FORMAT`) is in the current working directory and allow writting to arbitrary subdirectories if this is the case
@yannh
Copy link
Owner

yannh commented Jul 7, 2024

Hi @peschmae ! I'm not sure about adding this, It is opinionated (can only create subdirectories in the current working directory) and also can not create subdirectories of subdirectories. Would it not be easy to create the repository just before running this script if needed?

@peschmae
Copy link
Author

peschmae commented Jul 7, 2024

Hi @yannh
Thanks for the feedback.

I'm not sure about adding this, It is opinionated (can only create subdirectories in the current working directory) and also can not create subdirectories of subdirectories.

The current implementation, doesn't allow writting to any subdirectory, even if they already exists (due to the usage of os.path.basename), so even if we use something like FILENAME_FORMAT=existing/directory/{group}-{version} the files will always end up in the current working directory.
This was why I wanted to make it possible to write to a subdirectory at all.
The reason I opted to only allow subdirectories in the current working directory, is that the FILENAME_FORMAT is user input, and would need some sanitzing to ensure no OS files can be overwritten with this script. If only the current working directory is permitted, this will already limit the abuse potential quite a bit. (this is similar to what the current implementation is trying to achieve with os.path.basename )

I then came across #197 which tried to write the files into the directory structure used for the CRD Catalog (FILENAME_FORMAT= {fullgroup}/{kind}_{version}) which doesn't work either.
To also make this possible, I've added the code to create subdirectories. Since it uses parents=true it will create the full directory structure necessary. (even nested subdirectories eg FILENAME_FORMAT=this/doesnt/exist/yet/{group}-{version})

Would it not be easy to create the repository just before running this script if needed?

While it's possible to create a single subdirectory before running the script, the approach for FILENAME_FORMAT= {fullgroup}/{kind}_{version} would be a lot more complicated, as it would require to create all group directories previously, which entails handling the CRDs/schemas to extract the information. In the openapi2jsonschema all the relevant information is already present, and requires minimal changes.

@mhutter
Copy link

mhutter commented Oct 4, 2024

Hi @yannh and @peschmae,

I ran into this as well recently (also while preparing stuff for CRDs-Catalog).

My pain point was that I had to process a CRD definition that contained many different Groups, so manually creating them, AND then having to move all the generated files around would certainly be possible with a couple of lines of bash script, but certainly not worth my time :-) So I went for a simple 2-line change:

@@ -102,7 +102,9 @@ def write_schema_file(schema, filename):
     schemaJSON = json.dumps(schema, indent=2)

     # Dealing with user input here..
-    filename = os.path.basename(filename)
+    dirname = os.path.dirname(filename)
+    os.makedirs(dirname, exist_ok=True)
+
     f = open(filename, "w")
     print(schemaJSON, file=f)
     f.close()

I then ran the script like this:

FILENAME_FORMAT='{fullgroup}/{kind}_{version}' ./openapi2jsonschema.py path/to/crds.yaml

Yes of course if you set FILENAME_FORMAT to some system directory AND you run the script with root permissions, then you're f...ed. I don't entirely see the point of "some sanitzing to ensure no OS files can be overwritten with this script" here. After all, a local user will execute this script, not some random untrusted user.

@taylor-knapp
Copy link

@yannh seconding I manually updated the script locally to dynamically create directories based on the fullgroup.

I think it's worth allowing users to choose to add a subdirectory, even if there are some drawbacks. The alternative is now it 'fails' silently and just drops the directories.

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

Successfully merging this pull request may close these issues.

4 participants