Skip to content

Conversation

sameerzuberi
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Remove all dependency resolution logic from deployment_handler. This logic is now in a new dependency_resolver which will be called from within deployment_handler.
  • Refactor all deployment configuration logic into deployment_configuration. This makes it easier to get any necessary config values and streamlines deployment handler code.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

}

*root_ca_path = (char *) resp.data;
memcpy(root_ca_path->data, resp.data, resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to verify that the buffer is at least as long as resp.data before copying over the data. Similarly in other places

return ret;
}

assert(thing_name->len <= resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this assert is wrong. Should it not be thing_name->len > resp.len?

we should also fail gracefully in case the lengths do no match.
I think it can be done as such:

if(thing_name->len <= resp.len) {
    assert(false);
    return GGL_ERR_FAILURE;
}

This would allow us to fail gracefully even when the assert is not defined in production environments.

return ret;
}

assert(root_ca_path->len <= resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert also seems wrong no?

@rawalexe
Copy link
Member

PR #762 covers it

@rawalexe rawalexe closed this Feb 21, 2025
@archigup archigup deleted the refactor-deployments branch September 18, 2025 22:54
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