Skip to content

Adding AWS ECS task definition validation. #512

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

Conversation

djgoku
Copy link
Contributor

@djgoku djgoku commented Jan 9, 2025

Since #353 was merged this is now working! Thanks again for adding support in for #353 !

% check-jsonschema --builtin-schema vendor.amazon-ecs-intellisense-schema tests/example-files/hooks/positive/amazon-ecs-intellisense-schema/fargate.json
ok -- validation done
% check-jsonschema --builtin-schema vendor.amazon-ecs-intellisense-schema tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json
Schema validation errors were encountered.
  tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json::$: 'family' is a required property
  tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json::$: 'containerDefinitions' is a required property

@djgoku djgoku force-pushed the feature/add-vendor-aws-ecs-task-definition branch from e2222dc to 0177e56 Compare January 9, 2025 02:34
@djgoku djgoku force-pushed the feature/add-vendor-aws-ecs-task-definition branch from 0177e56 to c81ed4f Compare January 9, 2025 02:44
@sirosen
Copy link
Member

sirosen commented Jan 12, 2025

Hi there, thanks for the pull request!

Right now, I'm disinclined to accept this addition, but it may be possible for you to change my mind.

Issues:

  • the schema is not in schemastore (not a hard blocker, but a mark against it)
  • the user base for https://github.com/awslabs/amazon-ecs-intellisense-schema/ appears very small -- only 15 stars
  • awslabs in particular produces a lot of abandonware, and this project seems like it's mostly abandoned (it's getting minimal maintenance updates once a year, and has some very long update droughts)
  • there is no canonically correct filename pattern

I wish I had some more formal acceptance criteria for schemas (for me, so that I didn't have to make this a judgement call, and for you, so you could have clearer criteria to satisfy), but for now the rough rule is "if it's not in SchemaStore, make a convincing case for it".

Keeping in mind that a non-vendored schema can be used either by URL or by downloading it yourself, are you able to solve your particular use cases without this change? If not, why not (so that I better understand usages)?


If you want to push this forward, I think it would help to get the schema added to SchemaStore's catalog. They're very welcoming over there, so it shouldn't be hard unless there's some blocking issue with the schema. That way, I could at least keep to the rule that check-jsonschema's vendoring tracks SchemaStore.

@djgoku
Copy link
Contributor Author

djgoku commented Jan 12, 2025

@sirosen, I completely understand your perspective! I concur with all your points.

I also want to express my gratitude for your work on #353, as it would not have been possible without it.

Here’s a bit of a clue for anyone searching the repository for a similar issue:

Happy Path:

% check-jsonschema --schemafile https://raw.githubusercontent.com/awslabs/amazon-ecs-intellisense-schema/mainline/src/model/schema/schema.json tests/example-files/hooks/positive/amazon-ecs-intellisense-schema/fargate.json 
ok -- validation done

Not happy path:

% check-jsonschema --schemafile https://raw.githubusercontent.com/awslabs/amazon-ecs-intellisense-schema/mainline/src/model/schema/schema.json tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json 
Schema validation errors were encountered.
  tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json::$: 'family' is a required property
  tests/example-files/hooks/negative/amazon-ecs-intellisense-schema/invalid.json::$: 'containerDefinitions' is a required property

@djgoku djgoku closed this Jan 12, 2025
@djgoku djgoku deleted the feature/add-vendor-aws-ecs-task-definition branch January 12, 2025 16:35
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.

2 participants